root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Add latest changes from gitlab-org/gitlab@master

This commit is contained in:
GitLab Bot 2023-01-10 18:07:39 +00:00
parent 87f8fdb93c
commit 8cc4a6f23d
98 changed files with 1818 additions and 1405 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
.gitlab/ci/rails.gitlab-ci.yml
View File

@ -727,12 +727,6 @@ rspec-ee system pg11:
- .rspec-ee-system-parallel
# PG12
rspec-ee unit pg12 es7:
extends:
- .rspec-ee-base-pg12-es7
- .rspec-ee-unit-parallel
- .rails:rules:default-branch-schedule-nightly--code-backstage-ee-only
rspec-ee unit pg12 opensearch1:
extends:
- .rspec-ee-base-pg12-opensearch1
@ -745,12 +739,6 @@ rspec-ee unit pg12 opensearch2:
- .rspec-ee-unit-parallel
- .rails:rules:default-branch-schedule-nightly--code-backstage-ee-only
rspec-ee integration pg12 es7:
extends:
- .rspec-ee-base-pg12-es7
- .rspec-ee-integration-parallel
- .rails:rules:default-branch-schedule-nightly--code-backstage-ee-only
rspec-ee integration pg12 opensearch1:
extends:
- .rspec-ee-base-pg12-opensearch1
@ -763,12 +751,6 @@ rspec-ee integration pg12 opensearch2:
- .rspec-ee-integration-parallel
- .rails:rules:default-branch-schedule-nightly--code-backstage-ee-only
rspec-ee system pg12 es7:
extends:
- .rspec-ee-base-pg12-es7
- .rspec-ee-system-parallel
- .rails:rules:default-branch-schedule-nightly--code-backstage-ee-only
rspec-ee system pg12 opensearch1:
extends:
- .rspec-ee-base-pg12-opensearch1

6
.gitlab/ci/rails/shared.gitlab-ci.yml
View File

@ -120,12 +120,6 @@ include:
- .rspec-base
- .use-pg12-es7-ee
.rspec-ee-base-pg12-es7:
extends:
- .rspec-base
- .use-pg12-es7-ee
- .rails:rules:run-search-tests
.rspec-ee-base-pg12-es8:
extends:
- .rspec-base

7
app/assets/javascripts/abuse_reports/components/abuse_category_selector.vue
View File

@ -72,7 +72,12 @@ export default {
<input :value="$options.csrf.token" type="hidden" name="authenticity_token" />
<input type="hidden" name="user_id" :value="userId" data-testid="input-user-id" />
<input type="hidden" name="ref_url" :value="reportedFromUrl" data-testid="input-referer" />
<input
type="hidden"
name="abuse_report[reported_from_url]"
:value="reportedFromUrl"
data-testid="input-referer"
/>
<gl-form-group :label="$options.i18n.label">
<gl-form-radio-group

3
app/assets/javascripts/behaviors/shortcuts/shortcut.vue
View File

@ -1,4 +1,5 @@
<script>
import { getModifierKey } from '~/constants';
import { __, s__ } from '~/locale';
// Map some keys to their proper representation depending on the system
@ -22,7 +23,7 @@ const getKeyMap = () => {
keyMap.alt = keyMap.option;
// Mod is Command on Mac, and Ctrl on Windows/Linux
keyMap.mod = window.gl?.client?.isMac ? keyMap.command : keyMap.ctrl;
keyMap.mod = getModifierKey(true);
return keyMap;
};

7
app/assets/javascripts/constants.js
View File

@ -1,3 +1,6 @@
import { s__ } from '~/locale';
/* eslint-disable @gitlab/require-i18n-strings */
export const MODIFIER_KEY = window.gl?.client?.isMac ? '⌘' : s__('KeyboardKey|Ctrl+');
export const getModifierKey = (removeSuffix = false) => {
const winKey = `Ctrl${removeSuffix ? '' : '+'}`;
return window.gl?.client?.isMac ? '⌘' : winKey;
};

4
app/assets/javascripts/diffs/components/tree_list.vue
View File

@ -2,11 +2,13 @@
import { GlTooltipDirective, GlIcon } from '@gitlab/ui';
import { mapActions, mapGetters, mapState } from 'vuex';
import micromatch from 'micromatch';
import { MODIFIER_KEY } from '~/constants';
import { getModifierKey } from '~/constants';
import { s__, sprintf } from '~/locale';
import FileTree from '~/vue_shared/components/file_tree.vue';
import DiffFileRow from './diff_file_row.vue';
const MODIFIER_KEY = getModifierKey();
export default {
directives: {
GlTooltip: GlTooltipDirective,

12
app/assets/javascripts/editor/constants.js
View File

@ -1,7 +1,9 @@
import { MODIFIER_KEY } from '~/constants';
import { getModifierKey } from '~/constants';
import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants';
import { s__, __, sprintf } from '~/locale';
const modifierKey = getModifierKey();
export const URI_PREFIX = 'gitlab';
export const CONTENT_UPDATE_DEBOUNCE = DEFAULT_DEBOUNCE_AND_THROTTLE_MS;
@ -67,7 +69,7 @@ export const EXTENSION_MARKDOWN_BUTTONS = [
{
id: 'bold',
label: sprintf(s__('MarkdownEditor|Add bold text (%{modifierKey}B)'), {
modifierKey: MODIFIER_KEY,
modifierKey,
}),
data: {
mdTag: '**',
@ -77,7 +79,7 @@ export const EXTENSION_MARKDOWN_BUTTONS = [
{
id: 'italic',
label: sprintf(s__('MarkdownEditor|Add italic text (%{modifierKey}I)'), {
modifierKey: MODIFIER_KEY,
modifierKey,
}),
data: {
mdTag: '_',
@ -87,7 +89,7 @@ export const EXTENSION_MARKDOWN_BUTTONS = [
{
id: 'strikethrough',
label: sprintf(s__('MarkdownEditor|Add strikethrough text (%{modifierKey}⇧X)'), {
modifierKey: MODIFIER_KEY,
modifierKey,
}),
data: {
mdTag: '~~',
@ -113,7 +115,7 @@ export const EXTENSION_MARKDOWN_BUTTONS = [
{
id: 'link',
label: sprintf(s__('MarkdownEditor|Add a link (%{modifier_key}K)'), {
modifierKey: MODIFIER_KEY,
modifierKey,
}),
data: {
mdTag: '[{text}](url)',

2
app/assets/javascripts/packages_and_registries/container_registry/explorer/components/details_page/tags_list_row.vue
View File

@ -95,7 +95,7 @@ export default {
return formatDate(this.tag.createdAt, 'isoDate');
},
publishedTime() {
return formatDate(this.tag.createdAt, 'hh:MM Z');
return formatDate(this.tag.createdAt, 'HH:MM:ss Z');
},
formattedRevision() {
// to be removed when API response is adjusted

8
app/assets/javascripts/packages_and_registries/infrastructure_registry/details/components/package_history.vue
View File

@ -96,9 +96,9 @@ export default {
<history-item icon="commit" data-testid="first-pipeline-commit">
<gl-sprintf :message="$options.i18n.createdByCommitText">
<template #link>
<gl-link :href="firstPipeline.project.commit_url"
>#{{ truncate(firstPipeline.sha) }}</gl-link
>
<gl-link :href="firstPipeline.project.commit_url">{{
truncate(firstPipeline.sha)
}}</gl-link>
</template>
<template #branch>
<strong>{{ firstPipeline.ref }}</strong>
@ -147,7 +147,7 @@ export default {
>
<gl-sprintf :message="$options.i18n.combinedUpdateText">
<template #link>
<gl-link :href="pipeline.project.commit_url">#{{ truncate(pipeline.sha) }}</gl-link>
<gl-link :href="pipeline.project.commit_url">{{ truncate(pipeline.sha) }}</gl-link>
</template>
<template #branch>
<strong>{{ pipeline.ref }}</strong>

12
app/assets/javascripts/packages_and_registries/package_registry/components/details/package_history.vue
View File

@ -159,9 +159,9 @@ export default {
<history-item icon="commit" data-testid="first-pipeline-commit">
<gl-sprintf :message="$options.i18n.createdByCommitText">
<template #link>
<gl-link :href="firstPipeline.commitPath" @click="trackCommitClick"
>#{{ truncate(firstPipeline.sha) }}</gl-link
>
<gl-link :href="firstPipeline.commitPath" @click="trackCommitClick">{{
truncate(firstPipeline.sha)
}}</gl-link>
</template>
<template #branch>
<strong>{{ firstPipeline.ref }}</strong>
@ -212,9 +212,9 @@ export default {
>
<gl-sprintf :message="$options.i18n.combinedUpdateText">
<template #link>
<gl-link :href="pipeline.commitPath" @click="trackCommitClick"
>#{{ truncate(pipeline.sha) }}</gl-link
>
<gl-link :href="pipeline.commitPath" @click="trackCommitClick">{{
truncate(pipeline.sha)
}}</gl-link>
</template>
<template #branch>
<strong>{{ pipeline.ref }}</strong>

5
app/assets/javascripts/users/profile/components/report_abuse_button.vue
View File

@ -1,6 +1,7 @@
<script>
import { GlButton, GlTooltipDirective } from '@gitlab/ui';
import { __ } from '~/locale';
import { BV_HIDE_TOOLTIP } from '~/lib/utils/constants';
import AbuseCategorySelector from '~/abuse_reports/components/abuse_category_selector.vue';
@ -34,6 +35,9 @@ export default {
closeDrawer() {
this.open = false;
},
hideTooltips() {
this.$root.$emit(BV_HIDE_TOOLTIP);
},
},
};
</script>
@ -45,6 +49,7 @@ export default {
:aria-label="buttonTooltipText"
icon="error"
@click="openDrawer"
@mouseout="hideTooltips"
/>
<abuse-category-selector :show-drawer="open" @close-drawer="closeDrawer" />
</span>

1
app/assets/javascripts/work_items/components/work_item_notes.vue
View File

@ -209,6 +209,7 @@ export default {
:query-variables="queryVariables"
:full-path="fullPath"
:work-item-id="workItemId"
:fetch-by-iid="fetchByIid"
@error="$emit('error', $event)"
/>
</ul>

27
app/controllers/abuse_reports_controller.rb
View File

@ -1,20 +1,22 @@
# frozen_string_literal: true
class AbuseReportsController < ApplicationController
before_action :set_user, :set_ref_url, only: [:new, :add_category]
before_action :save_origin_url, only: [:new, :add_category]
before_action :set_origin_url, only: [:create]
before_action :set_user, only: [:new, :add_category]
feature_category :insider_threat
def new
@abuse_report = AbuseReport.new(user_id: @user.id)
@abuse_report = AbuseReport.new(
user_id: @user.id,
reported_from_url: params.fetch(:ref_url, '')
)
end
def add_category
@abuse_report = AbuseReport.new(
user_id: @user.id,
category: report_params[:category]
category: report_params[:category],
reported_from_url: report_params[:reported_from_url]
)
render :new
@ -39,7 +41,7 @@ class AbuseReportsController < ApplicationController
private
def report_params
params.require(:abuse_report).permit(:message, :user_id, :category)
params.require(:abuse_report).permit(:message, :user_id, :category, :reported_from_url)
end
# rubocop: disable CodeReuse/ActiveRecord
@ -53,17 +55,4 @@ class AbuseReportsController < ApplicationController
end
end
# rubocop: enable CodeReuse/ActiveRecord
def set_ref_url
@ref_url = params.fetch(:ref_url, '')
end
def save_origin_url
@origin_url = params.fetch(:ref_url, request.referer)
session[:abuse_report_origin_url] = @origin_url
end
def set_origin_url
@origin_url = session[:abuse_report_origin_url]
end
end

19
app/models/abuse_report.rb
View File

@ -17,10 +17,19 @@ class AbuseReport < ApplicationRecord
uniqueness: {
scope: :reporter_id,
message: ->(object, data) do
_('You have already reported this user')
_('has already been reported for abuse')
end
}
validates :reported_from_url,
allow_blank: true,
length: { maximum: 512 },
addressable_url: {
dns_rebind_protection: true,
blocked_message: 'is an invalid URL. You can try reporting the abuse again, ' \
'or contact a GitLab administrator for help.'
}
scope :by_user, ->(user) { where(user_id: user) }
scope :with_users, -> { includes(:reporter, :user) }
@ -38,6 +47,14 @@ class AbuseReport < ApplicationRecord
# For CacheMarkdownField
alias_method :author, :reporter
HUMANIZED_ATTRIBUTES = {
reported_from_url: "Reported from"
}.freeze
def self.human_attribute_name(attr, options = {})
HUMANIZED_ATTRIBUTES[attr.to_sym] || super
end
def remove_user(deleted_by:)
user.delete_async(deleted_by: deleted_by, params: { hard_delete: true })
end

9
app/models/concerns/project_features_compatibility.rb
View File

@ -124,8 +124,13 @@ module ProjectFeaturesCompatibility
private
def write_feature_attribute_boolean(field, value)
access_level = Gitlab::Utils.to_boolean(value) ? ProjectFeature::ENABLED : ProjectFeature::DISABLED
write_feature_attribute_raw(field, access_level)
value_type = Gitlab::Utils.to_boolean(value)
if value_type.in?([true, false])
access_level = value_type ? ProjectFeature::ENABLED : ProjectFeature::DISABLED
write_feature_attribute_raw(field, access_level)
else
write_feature_attribute_string(field, value)
end
end
def write_feature_attribute_string(field, value)

20
app/models/personal_access_token.rb
View File

@ -21,6 +21,11 @@ class PersonalAccessToken < ApplicationRecord
after_initialize :set_default_scopes, if: :persisted?
before_save :ensure_token
# During the implementation of Admin Mode for API, tokens of
# administrators should automatically get the `admin_mode` scope as well
# See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
before_create :add_admin_mode_scope, if: :user_admin?
scope :active, -> { not_revoked.not_expired }
scope :expiring_and_not_notified, ->(date) { where(["revoked = false AND expire_notification_delivered = false AND expires_at >= CURRENT_DATE AND expires_at <= ?", date]) }
scope :expired_today_and_not_notified, -> { where(["revoked = false AND expires_at = CURRENT_DATE AND after_expiry_notification_delivered = false"]) }
@ -79,7 +84,12 @@ class PersonalAccessToken < ApplicationRecord
protected
def validate_scopes
unless revoked || scopes.all? { |scope| Gitlab::Auth.all_available_scopes.include?(scope.to_sym) }
# During the implementation of Admin Mode for API,
# the `admin_mode` scope is not yet part of `all_available_scopes` but still valid.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
valid_scopes = Gitlab::Auth.all_available_scopes + [Gitlab::Auth::ADMIN_MODE_SCOPE]
unless revoked || scopes.all? { |scope| valid_scopes.include?(scope.to_sym) }
errors.add :scopes, "can only contain available scopes"
end
end
@ -91,6 +101,14 @@ class PersonalAccessToken < ApplicationRecord
self.scopes = Gitlab::Auth::DEFAULT_SCOPES if self.scopes.empty?
end
def user_admin?
user.admin? # rubocop: disable Cop/UserAdmin
end
def add_admin_mode_scope
self.scopes += [Gitlab::Auth::ADMIN_MODE_SCOPE.to_s]
end
end
PersonalAccessToken.prepend_mod_with('PersonalAccessToken')

6
app/services/projects/create_service.rb
View File

@ -22,6 +22,12 @@ module Projects
end
def execute
params[:wiki_enabled] = params[:wiki_access_level] if params[:wiki_access_level]
params[:builds_enabled] = params[:builds_access_level] if params[:builds_access_level]
params[:snippets_enabled] = params[:builds_access_level] if params[:snippets_access_level]
params[:merge_requests_enabled] = params[:merge_requests_access_level] if params[:merge_requests_access_level]
params[:issues_enabled] = params[:issues_access_level] if params[:issues_access_level]
if create_from_template?
return ::Projects::CreateFromTemplateService.new(current_user, params).execute
end

46
app/views/abuse_reports/new.html.haml
View File

@ -1,30 +1,36 @@
- page_title _("Report abuse to administrator")
%h1.page-title.gl-font-size-h-display
= _("Report abuse to administrator")
%p
= _("Use this form to report to the administrator users who create spam issues, comments or behave inappropriately.")
%p
= _("A member of the abuse team will review your report as soon as possible.")
%hr
= gitlab_ui_form_for @abuse_report, html: { class: 'js-quick-submit js-requires-input'} do |f|
= form_errors(@abuse_report, custom_message: [:user_id])
.row
.col-lg-8
%h1.page-title.gl-font-size-h-display
= _("Report abuse to administrator")
%p
= _("Please use this form to report to the administrator users who create spam issues, comments or behave inappropriately.")
= _("A member of the abuse team will review your report as soon as possible.")
= gitlab_ui_form_for @abuse_report, html: { class: 'js-quick-submit'} do |f|
= form_errors(@abuse_report)
= f.hidden_field :user_id
= f.hidden_field :category
.form-group.row
.col-sm-2.col-form-label
= f.label :user_id
.col-sm-10
.col-lg-8
= f.label :reported_user
- name = "#{@abuse_report.user.name} (@#{@abuse_report.user.username})"
= text_field_tag :user_name, name, class: "form-control", readonly: true
.form-group.row
.col-sm-2.col-form-label
= f.label :message
.col-sm-10
= f.text_area :message, class: "form-control", rows: 2, required: true, value: sanitize(@ref_url)
.col-lg-8
= f.label :reported_from
= f.text_field :reported_from_url, class: "form-control", readonly: true
.form-group.row
.col-lg-8
= f.label :reason
= f.text_area :message, class: "form-control", rows: 4, required: true
.form-text.text-muted
= _("Explain the problem. If appropriate, provide a link to the relevant issue or comment.")
= _("Explain why you're reporting the user.")
.form-actions
= f.submit _("Send report"), pajamas_button: true
= link_to _('Cancel'), @origin_url, class: "gl-button btn btn-default btn-cancel"
= render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm) do
= _('Send report')
= render Pajamas::ButtonComponent.new(href: @abuse_report.reported_from_url, button_options: { class: 'gl-ml-3' }) do
= _('Cancel')

2
app/views/users/show.html.haml
View File

@ -18,7 +18,7 @@
icon: 'pencil',
button_options: { class: 'gl-flex-grow-1 gl-mx-1 has-tooltip', title: s_('UserProfile|Edit profile'), 'aria-label': 'Edit profile', data: { toggle: 'tooltip', placement: 'bottom', container: 'body' }})
- elsif current_user
#js-report-abuse{ data: { form_submit_path: add_category_abuse_reports_path, user_id: @user.id, reported_from_url: request.referer || user_url(@user) } }
#js-report-abuse{ data: { form_submit_path: add_category_abuse_reports_path, user_id: @user.id, reported_from_url: user_url(@user) } }
- verified_gpg_keys = @user.gpg_keys.select(&:verified?)
- if verified_gpg_keys.any?
= render Pajamas::ButtonComponent.new(href: user_gpg_keys_path,

8
config/feature_flags/development/repository_archive_hotlinking_interception.yml
View File

@ -1,8 +0,0 @@
---
name: repository_archive_hotlinking_interception
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/commit/50c11f278d18fe1f3fb12eb595067216bb58ade2
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/369433
milestone: '12.10'
type: development
group: group::source code
default_enabled: true

12
config/feature_flags/development/rate_limit_gitlab_shell.yml → config/feature_flags/development/sec_mark_dropped_findings_as_resolved_scheduler.yml
View File

@ -1,8 +1,8 @@
---
name: rate_limit_gitlab_shell
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78373
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350465
milestone: '14.7'
name: sec_mark_dropped_findings_as_resolved_scheduler
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108486
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/387577
milestone: '15.8'
type: development
group: group::source code
default_enabled: true
group: group::static analysis
default_enabled: false

21
db/migrate/20221212054640_add_reported_from_to_abuse_reports.rb Normal file
View File

@ -0,0 +1,21 @@
# frozen_string_literal: true
class AddReportedFromToAbuseReports < Gitlab::Database::Migration[2.1]
disable_ddl_transaction!
def up
with_lock_retries do
unless column_exists?(:abuse_reports, :reported_from_url)
add_column :abuse_reports, :reported_from_url, :text, null: false, default: ''
end
end
add_text_limit :abuse_reports, :reported_from_url, 512
end
def down
with_lock_retries do
remove_column :abuse_reports, :reported_from_url if column_exists?(:abuse_reports, :reported_from_url)
end
end
end

21
db/post_migrate/20221228103133_queue_backfill_admin_mode_scope_for_personal_access_tokens.rb Normal file
View File

@ -0,0 +1,21 @@
# frozen_string_literal: true
class QueueBackfillAdminModeScopeForPersonalAccessTokens < Gitlab::Database::Migration[2.1]
MIGRATION = 'BackfillAdminModeScopeForPersonalAccessTokens'
DELAY_INTERVAL = 2.minutes
restrict_gitlab_migration gitlab_schema: :gitlab_main
def up
queue_batched_background_migration(
MIGRATION,
:personal_access_tokens,
:id,
job_interval: DELAY_INTERVAL
)
end
def down
delete_batched_background_migration(MIGRATION, :personal_access_tokens, :id, [])
end
end

1
db/schema_migrations/20221212054640 Normal file
View File

@ -0,0 +1 @@
18c98815e882f808ec2d5d29d605b89bd725690f0c399627eaa98f4ff7d3ef76

1
db/schema_migrations/20221228103133 Normal file
View File

@ -0,0 +1 @@
59e19291b3f8bb08dd63c1b1993af8f75e06d56ca776c3e8711adcc8c5c26e86

4
db/structure.sql
View File

@ -10603,7 +10603,9 @@ CREATE TABLE abuse_reports (
updated_at timestamp without time zone,
message_html text,
cached_markdown_version integer,
category smallint DEFAULT 1 NOT NULL
category smallint DEFAULT 1 NOT NULL,
reported_from_url text DEFAULT ''::text NOT NULL,
CONSTRAINT check_ab1260fa6c CHECK ((char_length(reported_from_url) <= 512))
);
CREATE SEQUENCE abuse_reports_id_seq

2
doc/administration/instance_limits.md
View File

@ -912,7 +912,7 @@ An upper and lower limit applies to each of these:
The lower limits result in additional diffs being collapsed. The higher limits
prevent any more changes from rendering. For more information about these limits,
[read the development documentation](../development/diffs.md#diff-limits).
[read the development documentation](../development/merge_request_concepts/diffs/index.md#diff-limits).
### Merge request reports size limit

2
doc/administration/merge_request_diffs.md
View File

@ -1,6 +1,6 @@
---
stage: Create
group: Editor
group: Source Code
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
---

6
doc/administration/pages/index.md
View File

@ -185,7 +185,7 @@ you must also add the full paths as shown below:
1. [Reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure).
1. If you're using [Pages Access Control](#access-control), update the redirect URI in the GitLab Pages
[System OAuth application](../../integration/oauth_provider.md#instance-wide-applications)
[System OAuth application](../../integration/oauth_provider.md#create-an-instance-wide-application)
to use the HTTPS protocol.
WARNING:
@ -384,7 +384,7 @@ then you need to also add the full paths as shown below:
1. [Reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure).
1. If you're using [Pages Access Control](#access-control), update the redirect URI in the GitLab Pages
[System OAuth application](../../integration/oauth_provider.md#instance-wide-applications)
[System OAuth application](../../integration/oauth_provider.md#create-an-instance-wide-application)
to use the HTTPS protocol.
### Custom domain verification
@ -1428,7 +1428,7 @@ Once added, reconfigure with `sudo gitlab-ctl reconfigure` and restart GitLab wi
You may see this error if `pages_external_url` was updated at some point of time. Verify the following:
1. The **Callback URL**/Redirect URI in the GitLab Pages [System OAuth application](../../integration/oauth_provider.md#instance-wide-applications)
1. The **Callback URL**/Redirect URI in the GitLab Pages [System OAuth application](../../integration/oauth_provider.md#create-an-instance-wide-application)
is using the protocol (HTTP or HTTPS) that `pages_external_url` is configured to use.
1. The domain and path components of `Redirect URI` are valid: they should look like `projects.<pages_external_url>/auth`.

2
doc/administration/pages/source.md
View File

@ -445,7 +445,7 @@ Pages access control is disabled by default. To enable it:
```
1. [Restart GitLab](../restart_gitlab.md#installations-from-source).
1. Create a new [system OAuth application](../../integration/oauth_provider.md#user-owned-applications).
1. Create a new [system OAuth application](../../integration/oauth_provider.md#create-a-user-owned-application).
This should be called `GitLab Pages` and have a `Redirect URL` of
`https://projects.example.io/auth`. It does not need to be a "trusted"
application, but it does need the `api` scope.

2
doc/api/merge_requests.md
View File

@ -986,7 +986,7 @@ returned by the API or viewed via the UI. When these limits impact the results,
field contains a value of `true`. Diff data without these limits applied can be retrieved by
adding the `access_raw_diffs` parameter, accessing diffs not from the database but from Gitaly directly.
This approach is generally slower and more resource-intensive, but isn't subject to size limits
placed on database-backed diffs. [Limits inherent to Gitaly](../development/diffs.md#diff-limits)
placed on database-backed diffs. [Limits inherent to Gitaly](../development/merge_request_concepts/diffs/index.md#diff-limits)
still apply.
```plaintext

8
doc/api/oauth2.md
View File

@ -59,7 +59,7 @@ resources which the `application` can access. Upon creation, you obtain the
**must be kept secure**. It is also advantageous to keep the _Application ID_
secret when your application architecture allows.
For a list of scopes in GitLab, see [the provider documentation](../integration/oauth_provider.md#authorized-applications).
For a list of scopes in GitLab, see [the provider documentation](../integration/oauth_provider.md#view-all-authorized-applications).
### Prevent CSRF attacks
@ -116,7 +116,7 @@ Before starting the flow, generate the `STATE`, the `CODE_VERIFIER` and the `COD
This page asks the user to approve the request from the app to access their
account based on the scopes specified in `REQUESTED_SCOPES`. The user is then
redirected back to the specified `REDIRECT_URI`. The [scope parameter](../integration/oauth_provider.md#authorized-applications)
redirected back to the specified `REDIRECT_URI`. The [scope parameter](../integration/oauth_provider.md#view-all-authorized-applications)
is a space-separated list of scopes associated with the user.
For example,`scope=read_user+profile` requests the `read_user` and `profile` scopes.
The redirect includes the authorization `code`, for example:
@ -196,7 +196,7 @@ be used as a CSRF token.
This page asks the user to approve the request from the app to access their
account based on the scopes specified in `REQUESTED_SCOPES`. The user is then
redirected back to the specified `REDIRECT_URI`. The [scope parameter](../integration/oauth_provider.md#authorized-applications)
redirected back to the specified `REDIRECT_URI`. The [scope parameter](../integration/oauth_provider.md#view-all-authorized-applications)
is a space-separated list of scopes associated with the user.
For example,`scope=read_user+profile` requests the `read_user` and `profile` scopes.
The redirect includes the authorization `code`, for example:
@ -352,7 +352,7 @@ curl --header "Authorization: Bearer OAUTH-TOKEN" "https://gitlab.example.com/ap
## Access Git over HTTPS with `access token`
A token with [scope](../integration/oauth_provider.md#authorized-applications)
A token with [scope](../integration/oauth_provider.md#view-all-authorized-applications)
`read_repository` or `write_repository` can access Git over HTTPS. Use the token as the password.
The username must be `oauth2`, not your username:

2
doc/api/repositories.md
View File

@ -171,7 +171,7 @@ curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.com/api/v4/pr
This endpoint can be accessed without authentication if the repository is
publicly accessible. Diffs can have an empty diff string if
[diff limits](../development/diffs.md#diff-limits) are reached.
[diff limits](../development/merge_request_concepts/diffs/index.md#diff-limits) are reached.
```plaintext
GET /projects/:id/repository/compare

7
doc/api/secure_files.md
View File

@ -7,11 +7,10 @@ type: reference, api
# Project-level Secure Files API **(FREE)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78227) in GitLab 14.8. [Deployed behind the `ci_secure_files` flag](../administration/feature_flags.md), disabled by default.
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78227) in GitLab 14.8. [Deployed behind the `ci_secure_files` flag](../administration/feature_flags.md), disabled by default.
> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/350748) in GitLab 15.7.
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 `ci_secure_files`. Limited to 100 secure files per project. Files must be smaller than 5 MB. The feature is not ready for production use.
Limited to 100 secure files per project. Files must be smaller than 5 MB. Project-level Secure Files is an experimental feature developed by [GitLab Incubation Engineering](https://about.gitlab.com/handbook/engineering/incubation/).
## List project secure files

16
doc/ci/secure_files/index.md
View File

@ -7,24 +7,20 @@ type: reference
# Project-level Secure Files **(FREE)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78227) in GitLab 14.8. [Deployed behind the `ci_secure_files` flag](../../administration/feature_flags.md), disabled by default.
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78227) in GitLab 14.8. [Deployed behind the `ci_secure_files` flag](../../administration/feature_flags.md), disabled by default.
> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/350748) in GitLab 15.7.
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 `ci_secure_files`. Limited to 100 secure files per project. Files must be smaller
than 5 MB. Project-level Secure Files is an experimental feature developed by [GitLab Incubation Engineering](https://about.gitlab.com/handbook/engineering/incubation/).
Project-level Secure Files is still in development, but you can:
Project-level Secure Files is an experimental feature developed by [GitLab Incubation Engineering](https://about.gitlab.com/handbook/engineering/incubation/).
The feature is still in development, but you can:
- [Request a feature](https://gitlab.com/gitlab-org/incubation-engineering/mobile-devops/feedback/-/issues/new?issuable_template=feature_request).
- [Report a bug](https://gitlab.com/gitlab-org/incubation-engineering/mobile-devops/feedback/-/issues/new?issuable_template=report_bug).
- [Share feedback](https://gitlab.com/gitlab-org/incubation-engineering/mobile-devops/feedback/-/issues/new?issuable_template=general_feedback).
You can securely store files for use in CI/CD pipelines as "secure files". These files
You can securely store up to 100 files for use in CI/CD pipelines as "secure files". These files
are stored securely outside of your project's repository, and are not version controlled.
It is safe to store sensitive information in these files. Secure files support both
plain text and binary file types.
plain text and binary file types, but must be 5 MB or less.
You can manage secure files in the project settings, or with the [secure files API](../../api/secure_files.md).

2
doc/development/api_graphql_styleguide.md
View File

@ -785,7 +785,7 @@ The documentation will mention that the old Global ID style is now deprecated.
## Mark schema items as Alpha
You can mark GraphQL schema items (fields, arguments, enum values, and mutations) as
[Alpha](https://about.gitlab.com/handbook/product/gitlab-the-product/#alpha-beta-ga).
[Alpha](../policy/alpha-beta-support.md#alpha-features).
An item marked as Alpha is [exempt from the deprecation process](#breaking-change-exemptions) and can be removed
at any time without notice. Mark an item as Alpha when it is

6
doc/development/cached_queries.md
View File

@ -37,9 +37,9 @@ in-memory objects whenever possible.
When you introduce a new feature, you should:
- Avoid N+1 queries.
- Minimize the [query count](merge_request_performance_guidelines.md#query-counts).
- Minimize the [query count](merge_request_concepts/performance.md#query-counts).
- Pay special attention to ensure
[cached queries](merge_request_performance_guidelines.md#cached-queries) are not
[cached queries](merge_request_concepts/performance.md#cached-queries) are not
masking N+1 problems.
## How to detect cached queries
@ -163,5 +163,5 @@ factors help improve the overall execution time:
## For more information
- [Metrics that would help us detect the potential N+1 Cached SQL calls](https://gitlab.com/gitlab-org/gitlab/-/issues/259007)
- [Merge request performance guidelines for cached queries](merge_request_performance_guidelines.md#cached-queries)
- [Merge request performance guidelines for cached queries](merge_request_concepts/performance.md#cached-queries)
- [Improvements for biggest offenders](https://gitlab.com/groups/gitlab-org/-/epics/4508)

4
doc/development/chatops_on_gitlabcom.md
View File

@ -22,12 +22,12 @@ To request access to ChatOps on GitLab.com:
1. Sign in to [Internal GitLab for Operations](https://ops.gitlab.net/users/sign_in)
with one of the following methods (Okta is not supported):
- The same username you use on GitLab.com. You may have to choose a different username later.
- The same username you use on GitLab.com.
- Selecting the **Sign in with Google** button to sign in with your GitLab.com email address.
1. Confirm that your username in [Internal GitLab for Operations](https://ops.gitlab.net/)
is the same as your username in [GitLab.com](https://gitlab.com/). If the usernames
don't match, update the username in [User Settings/Account for the Ops instance](https://ops.gitlab.net/-/profile/account).
don't match, update the username in [User Settings/Account for the Ops instance](https://ops.gitlab.net/-/profile/account). Matching usernames are required to reduce the administrative effort of running multiple platforms. Matching usernames also help with tasks like managing access requests and offboarding.
1. Comment in your onboarding issue, and tag your onboarding buddy and your manager.
Request they add you to the `ops` ChatOps project by running this command

2
doc/development/code_review.md
View File

@ -200,7 +200,7 @@ See the [test engineering process](https://about.gitlab.com/handbook/engineering
##### Performance, reliability, and availability
1. You are confident that this MR does not harm performance, or you have asked a reviewer to help assess the performance impact. ([Merge request performance guidelines](merge_request_performance_guidelines.md))
1. You are confident that this MR does not harm performance, or you have asked a reviewer to help assess the performance impact. ([Merge request performance guidelines](merge_request_concepts/performance.md))
1. You have added [information for database reviewers in the MR description](database_review.md#required), or you have decided that it is unnecessary.
- [Does this MR have database-related changes?](database_review.md)
1. You have considered the availability and reliability risks of this change.

4
doc/development/contributing/merge_request_workflow.md
View File

@ -195,7 +195,7 @@ the contribution acceptance criteria below:
and testing future changes.
1. Changes do not degrade performance:
- Avoid repeated polling of endpoints that require a significant amount of overhead.
- Check for N+1 queries via the SQL log or [`QueryRecorder`](../merge_request_performance_guidelines.md).
- Check for N+1 queries via the SQL log or [`QueryRecorder`](../merge_request_concepts/performance.md).
- Avoid repeated access of the file system.
- Use [polling with ETag caching](../polling.md) if needed to support real-time features.
1. If the merge request adds any new libraries (like gems or JavaScript libraries),
@ -223,7 +223,7 @@ requirements.
1. Working and clean code that is commented where needed.
1. The change is evaluated to [limit the impact of far-reaching work](https://about.gitlab.com/handbook/engineering/development/#reducing-the-impact-of-far-reaching-work).
1. [Performance guidelines](../merge_request_performance_guidelines.md) have been followed.
1. [Performance guidelines](../merge_request_concepts/performance.md) have been followed.
1. [Secure coding guidelines](https://gitlab.com/gitlab-com/gl-security/security-guidelines) have been followed.
1. [Application and rate limit guidelines](../merge_request_application_and_rate_limit_guidelines.md) have been followed.
1. [Documented](../documentation/index.md) in the `/doc` directory.

2
doc/development/database/pagination_guidelines.md
View File

@ -62,7 +62,7 @@ Offset-based pagination is the easiest way to paginate over records, however, it
- Avoid presenting total counts, prefer limit counts.
- Example: count maximum 1001 records, and then on the UI show 1000+ if the count is 1001, show the actual number otherwise.
- See the [badge counters approach](../merge_request_performance_guidelines.md#badge-counters) for more information.
- See the [badge counters approach](../merge_request_concepts/performance.md#badge-counters) for more information.
- Avoid using page numbers, use next and previous page buttons.
- Keyset pagination doesn't support page numbers.
- For APIs, advise against building URLs for the next page by "hand".

8
doc/development/database/query_recorder.md
View File

@ -10,7 +10,7 @@ QueryRecorder is a tool for detecting the [N+1 queries problem](https://guides.r
> Implemented in [spec/support/query_recorder.rb](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/support/helpers/query_recorder.rb) via [9c623e3e](https://gitlab.com/gitlab-org/gitlab-foss/commit/9c623e3e5d7434f2e30f7c389d13e5af4ede770a)
As a rule, merge requests [should not increase query counts](../merge_request_performance_guidelines.md#query-counts). If you find yourself adding something like `.includes(:author, :assignee)` to avoid having `N+1` queries, consider using QueryRecorder to enforce this with a test. Without this, a new feature which causes an additional model to be accessed can silently reintroduce the problem.
As a rule, merge requests [should not increase query counts](../merge_request_concepts/performance.md#query-counts). If you find yourself adding something like `.includes(:author, :assignee)` to avoid having `N+1` queries, consider using QueryRecorder to enforce this with a test. Without this, a new feature which causes an additional model to be accessed can silently reintroduce the problem.
## How it works
@ -52,7 +52,7 @@ there are no N+1 queries. Rather than make an extra request to warm the cache, p
## Cached queries
By default, QueryRecorder ignores [cached queries](../merge_request_performance_guidelines.md#cached-queries) in the count. However, it may be better to count
By default, QueryRecorder ignores [cached queries](../merge_request_concepts/performance.md#cached-queries) in the count. However, it may be better to count
all queries to avoid introducing an N+1 query that may be masked by the statement cache.
To do this, this requires the `:use_sql_query_cache` flag to be set.
You should pass the `skip_cached` variable to `QueryRecorder` and use the `exceed_all_query_limit` matcher:
@ -146,5 +146,5 @@ There are multiple ways to find the source of queries.
- [Bullet](../profiling.md#bullet) For finding `N+1` query problems
- [Performance guidelines](../performance.md)
- [Merge request performance guidelines - Query counts](../merge_request_performance_guidelines.md#query-counts)
- [Merge request performance guidelines - Cached queries](../merge_request_performance_guidelines.md#cached-queries)
- [Merge request performance guidelines - Query counts](../merge_request_concepts/performance.md#query-counts)
- [Merge request performance guidelines - Cached queries](../merge_request_concepts/performance.md#cached-queries)

2
doc/development/database_review.md
View File

@ -275,4 +275,4 @@ Include in the MR description:
- [Check query plans](database/understanding_explain_plans.md) and suggest improvements
to queries (changing the query, schema or adding indexes and similar)
- General guideline is for queries to come in below [100ms execution time](database/query_performance.md#timing-guidelines-for-queries)
- Avoid N+1 problems and minimize the [query count](merge_request_performance_guidelines.md#query-counts).
- Avoid N+1 problems and minimize the [query count](merge_request_concepts/performance.md#query-counts).

202
doc/development/diffs.md
View File

@ -1,199 +1,11 @@
---
stage: Create
group: Code Review
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
redirect_to: 'merge_request_concepts/diffs/index.md'
remove_date: '2023-04-10'
---
# Working with diffs
This document was moved to [another location](merge_request_concepts/diffs/index.md).
We rely on different sources to present diffs. These include:
- Gitaly service
- Database (through `merge_request_diff_files`)
- Redis (cached highlighted diffs)
## Deep Dive
<!-- vale gitlab.Spelling = NO -->
In January 2019, Oswaldo Ferreira hosted a Deep Dive (GitLab team members only:
`https://gitlab.com/gitlab-org/create-stage/issues/1`) on GitLab Diffs and Commenting on Diffs
functionality to share domain-specific knowledge with anyone who may work in this part of the
codebase in the future:
<!-- vale gitlab.Spelling = YES -->
- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i>
[Recording on YouTube](https://www.youtube.com/watch?v=K6G3gMcFyek)
- Slides on [Google Slides](https://docs.google.com/presentation/d/1bGutFH2AT3bxOPZuLMGl1ANWHqFnrxwQwjiwAZkF-TU/edit)
- [PDF slides](https://gitlab.com/gitlab-org/create-stage/uploads/b5ad2f336e0afcfe0f99db0af0ccc71a/)
Everything covered in this deep dive was accurate as of GitLab 11.7, and while specific details may
have changed since then, it should still serve as a good introduction.
## Architecture overview
### Merge request diffs
When refreshing a merge request (pushing to a source branch, force-pushing to target branch, or if the target branch now contains any commits from the MR)
we fetch the comparison information using `Gitlab::Git::Compare`, which fetches `base` and `head` data using Gitaly and diff between them through
`Gitlab::Git::Diff.between`.
The diffs fetching process _limits_ single file diff sizes and the overall size of the whole diff through a series of constant values. Raw diff files are
then persisted on `merge_request_diff_files` table.
Even though diffs larger than 10% of the value of `ApplicationSettings#diff_max_patch_bytes` are collapsed,
we still keep them on PostgreSQL. However, diff files larger than defined _safety limits_
(see the [Diff limits section](#diff-limits)) are _not_ persisted in the database.
In order to present diffs information on the merge request diffs page, we:
1. Fetch all diff files from database `merge_request_diff_files`
1. Fetch the _old_ and _new_ file blobs in batch to:
- Highlight old and new file content
- Know which viewer it should use for each file (text, image, deleted, etc)
- Know if the file content changed
- Know if it was stored externally
- Know if it had storage errors
1. If the diff file is cacheable (text-based), it's cached on Redis
using `Gitlab::Diff::FileCollection::MergeRequestDiff`
### Note diffs
When commenting on a diff (any comparison), we persist a truncated diff version
on `NoteDiffFile` (which is associated with the actual `DiffNote`). So instead
of hitting the repository every time we need the diff of the file, we:
1. Check whether we have the `NoteDiffFile#diff` persisted and use it
1. Otherwise, if it's a current MR revision, use the persisted
`MergeRequestDiffFile#diff`
1. In the last scenario, go the repository and fetch the diff
## Diff limits
As explained above, we limit single diff files and the size of the whole diff. There are scenarios where we collapse the diff file,
and cases where the diff file is not presented at all, and the user is guided to the Blob view.
### Diff collection limits
Limits that act onto all diff files collection. Files number, lines number and files size are considered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_files] = 100
```
File diffs are collapsed (but are expandable) if 100 files have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:safe_max_lines] = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000
```
File diffs are collapsed (but be expandable) if 5000 lines have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:safe_max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] * 5.kilobytes = 500.kilobytes
```
File diffs are collapsed (but be expandable) if 500 kilobytes have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:max_files] = Commit::DIFF_HARD_LIMIT_FILES = 1000
```
No more files are rendered at all if 1000 files have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:max_lines] = Commit::DIFF_HARD_LIMIT_LINES = 50000
```
No more files are rendered at all if 50,000 lines have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:max_files] * 5.kilobytes = 5000.kilobytes
```
No more files are rendered at all if 5 megabytes have already been rendered.
All collection limit parameters are sent and applied on Gitaly. That is, after the limit is surpassed,
Gitaly only returns the safe amount of data to be persisted on `merge_request_diff_files`.
### Individual diff file limits
Limits that act onto each diff file of a collection. Files number, lines number and files size are considered.
#### Expandable patches (collapsed)
Diff patches are collapsed when surpassing 10% of the value set in `ApplicationSettings#diff_max_patch_bytes`.
That is, it's equivalent to 10kb if the maximum allowed value is 100kb.
The diff is persisted and expandable if the patch size doesn't
surpass `ApplicationSettings#diff_max_patch_bytes`.
Although this nomenclature (Collapsing) is also used on Gitaly, this limit is only used on GitLab (hardcoded - not sent to Gitaly).
Gitaly only returns `Diff.Collapsed` (RPC) when surpassing collection limits.
#### Not expandable patches (too large)
The patch not be rendered if it's larger than `ApplicationSettings#diff_max_patch_bytes`.
Users see a `Changes are too large to be shown.` message and a button to view only that file in that commit.
```ruby
Commit::DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000
```
File diff is suppressed (technically different from collapsed, but behaves the same, and is expandable) if it has more than 5000 lines.
This limit is hardcoded and only applied on GitLab.
## Viewers
Diff Viewers, which can be found on `models/diff_viewer/*` are classes used to map metadata about each type of Diff File. It has information
whether it's a binary, which partial should be used to render it or which File extensions this class accounts for.
`DiffViewer::Base` validates _blobs_ (old and new versions) content, extension and file type to check if it can be rendered.
## Merge request diffs against the `HEAD` of the target branch
Historically, merge request diffs have been calculated by `git diff target...source` which compares the
`HEAD` of the source branch with the merge base (or a common ancestor) of the target branch and the source's.
This solution works well until the target branch starts containing some of the
changes introduced by the source branch: Consider the following case, in which the source branch
is `feature_a` and the target is `main`:
1. Checkout a new branch `feature_a` from `main` and remove `file_a` and `file_b` in it.
1. Add a commit that removes `file_a` to `main`.
The merge request diff still contains the `file_a` removal while the actual diff compared to
`main`'s `HEAD` has only the `file_b` removal. The diff with such redundant
changes is harder to review.
In order to display an up-to-date diff, in GitLab 12.9 we
[introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/27008) merge request
diffs compared against `HEAD` of the target branch: the
target branch is artificially merged into the source branch, then the resulting
merge ref is compared to the source branch to calculate an accurate
diff.
Until we complete the epics ["use merge refs for diffs"](https://gitlab.com/groups/gitlab-org/-/epics/854)
and ["merge conflicts in diffs"](https://gitlab.com/groups/gitlab-org/-/epics/4893),
both options `main (base)` and `main (HEAD)` are available to be displayed in merge requests:
![Merge ref head options](img/merge_ref_head_options_v13_6.png)
The `main (HEAD)` option is meant to replace `main (base)` in the future.
In order to support comments for both options, diff note positions are stored for
both `main (base)` and `main (HEAD)` versions ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/198457) in 12.10).
The position for `main (base)` version is stored in `Note#position` and
`Note#original_position` columns, for `main (HEAD)` version `DiffNotePosition`
has been introduced.
One of the key challenges to deal with when working on merge ref diffs are merge
conflicts. If the target and source branch contains a merge conflict, the branches
cannot be automatically merged. The
<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [recording on YouTube](https://www.youtube.com/watch?v=GFXIFA4ZuZw&feature=youtu.be&ab_channel=GitLabUnfiltered)
is a quick introduction to the problem and the motivation behind the [epic](https://gitlab.com/groups/gitlab-org/-/epics/854).
In 13.5 a solution for both-modified merge
conflict has been
[introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/232484). However,
there are more classes of merge conflicts that are to be
[addressed](https://gitlab.com/groups/gitlab-org/-/epics/4893) in the future.
<!-- This redirect file can be deleted after <2023-04-10>. -->
<!-- Redirects that point to other docs in the same project expire in three months. -->
<!-- Redirects that point to docs in a different project or site (for example, link is not relative and starts with `https:`) expire in one year. -->
<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/redirects.html -->

2
doc/development/documentation/workflow.md
View File

@ -21,7 +21,7 @@ If you are not a GitLab team member, or do not have the Developer role for the G
1. Select an [issue](https://about.gitlab.com/handbook/product/ux/technical-writing/#community-contribution-opportunities) you'd like to work on.
- You don't need an issue to open a merge request.
- For a Hackathon, in the issue, in a comment, mention the person who opened the issue and ask for the issue to be assigned to you.
- For a Hackathon, mention `@docs-hackathon` in a comment and ask for the issue to be assigned to you.
To be fair to other contributors, if you see someone has already asked to work on the issue, choose another issue.
If you are looking for issues to work on and don't see any that suit you, you can always fix [Vale](testing.md#vale) issues.
1. Go to the [GitLab repository](https://gitlab.com/gitlab-org/gitlab).

2
doc/development/feature_development.md
View File

@ -108,7 +108,7 @@ Consult these topics for information on contributing to specific GitLab features
- [Performance guidelines](performance.md) for writing code, benchmarks, and
certain patterns to avoid.
- [Caching guidelines](caching.md) for using caching in Rails under a GitLab environment.
- [Merge request performance guidelines](merge_request_performance_guidelines.md)
- [Merge request performance guidelines](merge_request_concepts/performance.md)
for ensuring merge requests do not negatively impact GitLab performance
- [Profiling](profiling.md) a URL or tracking down N+1 queries using Bullet.
- [Cached queries guidelines](cached_queries.md), for tracking down N+1 queries

15
doc/development/feature_flags/controls.md
View File

@ -328,6 +328,21 @@ If a project A has `:feature-set-1` enabled, there is no guarantee that project
For more detail, see [This is how percentages work in Flipper](https://www.hackwithpassion.com/this-is-how-percentages-work-in-flipper/).
### Verifying metrics after enabling feature flag
After turning on the feature flag, you need to [monitor the relevant graphs](https://about.gitlab.com/handbook/engineering/monitoring/) between each step:
1. Go to [`dashboards.gitlab.net`](https://dashboards.gitlab.net).
1. Turn on the `feature-flag`.
1. Watch `Latency: Apdex` for services that might be impacted by your change
(like `sidekiq service`, `api service` or `web service`). Then check out more in-depth
dashboards by selecting `Service Overview Dashboards` and choosing a dashboard that might
be related to your change.
In this illustration, you can see that the Apdex score started to decline after the feature flag was enabled at `09:46`. The feature flag was then deactivated at `10:31`, and the service returned to the original value:
![Feature Flag Metrics](../img/feature-flag-metrics.png)
### Feature flag change logging
#### ChatOps level

2
doc/development/go_guide/go_upgrade.md
View File

@ -193,7 +193,7 @@ be included in the relevant issues as part of the definition of done:
Upstream component maintainers must validate their Go-based projects using:
- Established unit tests in the codebase.
- Procedures established in [Merge Request Performance Guidelines](../merge_request_performance_guidelines.md).
- Procedures established in [Merge Request Performance Guidelines](../merge_request_concepts/performance.md).
- Procedures established in [Performance, Reliability, and Availability guidelines](../code_review.md#performance-reliability-and-availability).
Upstream component maintainers should consider validating their Go-based

BIN
doc/development/img/feature-flag-metrics.png Normal file
View File

Binary file not shown.
Side by Side

After

Width:  |  Height:  |  Size: 86 KiB

188
doc/development/merge_request_concepts/diffs/development.md Normal file
View File

@ -0,0 +1,188 @@
---
stage: Create
group: Code Review
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
---
# Merge request diffs development guide **(FREE)**
This document explains the backend design and flow of merge request diffs.
It should help contributors:
- Understand the code design.
- Identify areas for improvement through contribution.
It's intentional that it doesn't contain too many implementation details, as they
can change often. The code better explains these details. The components
mentioned here are the major parts of the application for how merge request diffs
are generated, stored, and returned to users.
NOTE:
This page is a living document. Update it accordingly when the parts
of the codebase touched in this document are changed or removed, or when new components
are added.
## Data model
Four main ActiveRecord models represent what we collectively refer to
as _diffs._ These database-backed records replicate data contained in the
project's Git repository, and are in part a cache against excessive access requests
to [Gitaly](../../gitaly.md). Additionally, they provide a logical place for:
- Calculated and retrieved metadata about the pieces of the diff.
- General class- and instance- based logic.
```mermaid
erDiagram
MergeRequest ||--|{ MergeRequestDiff: ""
MergeRequestDiff |{--|{ MergeRequestDiffCommit: ""
MergeRequestDiff |{--|| MergeRequestDiffDetail: ""
MergeRequestDiff |{--|{ MergeRequestDiffFile: ""
MergeRequestDiffCommit |{--|| MergeRequestDiffCommitUser: ""
```
### `MergeRequestDiff`
`MergeRequestDiff` is defined in `app/models/merge_request_diff.rb`. This
class holds metadata and context related to the diff resulting from a set of
commits. It defines methods that are the primary means for interacting with diff
contents, individual commits, and the files containing changes.
```ruby
#<MergeRequestDiff:0x00007fd1ed63b4d0
id: 28,
state: "collected",
merge_request_id: 28,
created_at: Tue, 06 Sep 2022 18:56:02.509469000 UTC +00:00,
updated_at: Tue, 06 Sep 2022 18:56:02.754201000 UTC +00:00,
base_commit_sha: "ae73cb07c9eeaf35924a10f713b364d32b2dd34f",
real_size: "9",
head_commit_sha: "bb5206fee213d983da88c47f9cf4cc6caf9c66dc",
start_commit_sha: "0b4bc9a49b562e85de7cc9e834518ea6828729b9",
commits_count: 6,
external_diff: "diff-28",
external_diff_store: 1,
stored_externally: nil,
files_count: 9,
sorted: true,
diff_type: "regular",
verification_checksum: nil>
```
Diff content is usually accessed through this class. Logic is often applied
to diff, file, and commit content before it is returned to a user.
### `MergeRequestDiffCommit`
`MergeRequestDiffCommit` is defined in `app/models/merge_request_diff_commit.rb`.
This class corresponds to a single commit contained in its corresponding `MergeRequestDiff`,
and holds header information about the commit.
```ruby
#<MergeRequestDiffCommit:0x00007fd1dfc6c4c0
authored_date: Wed, 06 Aug 2022 06:35:52.000000000 UTC +00:00,
committed_date: Wed, 06 Aug 2022 06:35:52.000000000 UTC +00:00,
merge_request_diff_id: 28,
relative_order: 0,
sha: "bb5206fee213d983da88c47f9cf4cc6caf9c66dc",
message: "Feature conflcit added\n\nSigned-off-by: Sample User <sample.user@example.com>\n",
trailers: {},
commit_author_id: 19,
committer_id: 19>
```
Every `MergeRequestDiffCommit` has a corresponding `MergeRequest::DiffCommitUser`
record it `:belongs_to`, in ActiveRecord parlance. These records are `:commit_author`
and `:committer`, and could be distinct individuals.
### `MergeRequest::DiffCommitUser`
`MergeRequest::DiffCommitUser` is defined in `app/models/merge_request/diff_commit_user.rb`.
It captures the `name` and `email` of a given commit, but contains no connection
itself to any `User` records.
```ruby
#<MergeRequest::DiffCommitUser:0x00007fd1dff7c930
id: 19,
name: "Sample User",
email: "sample.user@example.com">
```
### `MergeRequestDiffFile`
`MergeRequestDiffFile` is defined in `app/models/merge_request_diff_file.rb`.
This record of this class represents the diff of a single file contained in the
`MergeRequestDiff`. It holds both meta and specific information about the file's
relationship to the change, such as:
- Whether it is added or renamed.
- Its ordering in the diff.
- The raw diff output itself.
### `MergeRequestDiffDetail`
`MergeRequestDiffDetail` is defined in `app/models/merge_request_diff_detail.rb`.
This class provides verification information for Geo replication, but otherwise
is not used for user-facing diffs.
```ruby
#<MergeRequestDiffFile:0x00007fd1ef7c9048
merge_request_diff_id: 28,
relative_order: 0,
new_file: true,
renamed_file: false,
deleted_file: false,
too_large: false,
a_mode: "0",
b_mode: "100644",
new_path: "files/ruby/feature.rb",
old_path: "files/ruby/feature.rb",
diff:
"@@ -0,0 +1,4 @@\n+# This file was changed in feature branch\n+# We put different code here to make merge conflict\n+class Conflict\n+end\n",
binary: false,
external_diff_offset: nil,
external_diff_size: nil>
```
## Flow
These flowcharts should help explain the flow from the controllers down to the
models for different features. This page is not intended to document the entirety
of options for access and working with diffs, focusing solely on the most common.
### `batch_diffs.json`
The most common avenue for viewing diffs is the **Changes**
tab in the top navigation bar of merge request pages in the GitLab UI. When selected, the
diffs themselves are loaded via a paginated request to `/-/merge_requests/:id/batch_diffs.json`,
which is served by [`Projects::MergeRequests::DiffsController#diffs_batch`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/controllers/projects/merge_requests/diffs_controller.rb):
<!-- Don't delete the &nbsp; characters below. Mermaid returns a syntax error if they aren't included.-->
```mermaid
sequenceDiagram
Note over .#diffs_batch: Preload diffs and ivars
.#diffs_batch->>+.#define_diff_vars: &nbsp;
.#define_diff_vars ->>+ @merge_request: @merge_request_diffs =
Note right of @merge_request: An ordered collection of all diffs in MR
@merge_request-->>-.#define_diff_vars: &nbsp;
.#define_diff_vars ->>+ @merge_request: @merge_request_diff =
Note right of @merge_request: Most recent merge_request_diff (or commit)
@merge_request-->>-.#define_diff_vars: &nbsp;
.#define_diff_vars ->>+ .#define_diff_vars: @compare =
Note right of .#define_diff_vars:: param-filtered merge_request_diff(s)
.#define_diff_vars -->>- .#diffs_batch: &nbsp;
Note over .#diffs_batch: Preloading complete
.#diffs_batch->>+@merge_request: Calculate unfoldable diff lines
Note right of @merge_request: note_positions_for_paths.unfoldable
@merge_request-->>-.#diffs_batch: &nbsp;
Note over .#diffs_batch: Build options hash
Note over .#diffs_batch: Build cache_context
Note over .#diffs_batch: Unfold files in diff
.#diffs_batch->>+Gitlab_Diff_FileCollection_MergeRequestDiffBase: diffs.write_diff
Gitlab_Diff_FileCollection_MergeRequestDiffBase->>+Gitlab_Diff_HighlightCache: Highlight diff
Gitlab_Diff_HighlightCache -->>-Gitlab_Diff_FileCollection_MergeRequestDiffBase: Return highlighted diff
Note over Gitlab_Diff_FileCollection_MergeRequestDiffBase: Cache diff
Gitlab_Diff_FileCollection_MergeRequestDiffBase-->>-.#diffs_batch: &nbsp;
Note over .#diffs_batch: render JSON
```

199
doc/development/merge_request_concepts/diffs/index.md Normal file
View File

@ -0,0 +1,199 @@
---
stage: Create
group: Code Review
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
---
# Working with diffs
We rely on different sources to present diffs. These include:
- Gitaly service
- Database (through `merge_request_diff_files`)
- Redis (cached highlighted diffs)
## Deep Dive
<!-- vale gitlab.Spelling = NO -->
In January 2019, Oswaldo Ferreira hosted a Deep Dive (GitLab team members only:
`https://gitlab.com/gitlab-org/create-stage/issues/1`) on GitLab Diffs and Commenting on Diffs
functionality to share domain-specific knowledge with anyone who may work in this part of the
codebase in the future:
<!-- vale gitlab.Spelling = YES -->
- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i>
[Recording on YouTube](https://www.youtube.com/watch?v=K6G3gMcFyek)
- Slides on [Google Slides](https://docs.google.com/presentation/d/1bGutFH2AT3bxOPZuLMGl1ANWHqFnrxwQwjiwAZkF-TU/edit)
- [PDF slides](https://gitlab.com/gitlab-org/create-stage/uploads/b5ad2f336e0afcfe0f99db0af0ccc71a/)
Everything covered in this deep dive was accurate as of GitLab 11.7, and while specific details may
have changed since then, it should still serve as a good introduction.
## Architecture overview
### Merge request diffs
When refreshing a merge request (pushing to a source branch, force-pushing to target branch, or if the target branch now contains any commits from the MR)
we fetch the comparison information using `Gitlab::Git::Compare`, which fetches `base` and `head` data using Gitaly and diff between them through
`Gitlab::Git::Diff.between`.
The diffs fetching process _limits_ single file diff sizes and the overall size of the whole diff through a series of constant values. Raw diff files are
then persisted on `merge_request_diff_files` table.
Even though diffs larger than 10% of the value of `ApplicationSettings#diff_max_patch_bytes` are collapsed,
we still keep them on PostgreSQL. However, diff files larger than defined _safety limits_
(see the [Diff limits section](#diff-limits)) are _not_ persisted in the database.
In order to present diffs information on the merge request diffs page, we:
1. Fetch all diff files from database `merge_request_diff_files`
1. Fetch the _old_ and _new_ file blobs in batch to:
- Highlight old and new file content
- Know which viewer it should use for each file (text, image, deleted, etc)
- Know if the file content changed
- Know if it was stored externally
- Know if it had storage errors
1. If the diff file is cacheable (text-based), it's cached on Redis
using `Gitlab::Diff::FileCollection::MergeRequestDiff`
### Note diffs
When commenting on a diff (any comparison), we persist a truncated diff version
on `NoteDiffFile` (which is associated with the actual `DiffNote`). So instead
of hitting the repository every time we need the diff of the file, we:
1. Check whether we have the `NoteDiffFile#diff` persisted and use it
1. Otherwise, if it's a current MR revision, use the persisted
`MergeRequestDiffFile#diff`
1. In the last scenario, go the repository and fetch the diff
## Diff limits
As explained above, we limit single diff files and the size of the whole diff. There are scenarios where we collapse the diff file,
and cases where the diff file is not presented at all, and the user is guided to the Blob view.
### Diff collection limits
Limits that act onto all diff files collection. Files number, lines number and files size are considered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_files] = 100
```
File diffs are collapsed (but are expandable) if 100 files have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:safe_max_lines] = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000
```
File diffs are collapsed (but be expandable) if 5000 lines have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:safe_max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] * 5.kilobytes = 500.kilobytes
```
File diffs are collapsed (but be expandable) if 500 kilobytes have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:max_files] = Commit::DIFF_HARD_LIMIT_FILES = 1000
```
No more files are rendered at all if 1000 files have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:max_lines] = Commit::DIFF_HARD_LIMIT_LINES = 50000
```
No more files are rendered at all if 50,000 lines have already been rendered.
```ruby
Gitlab::Git::DiffCollection.collection_limits[:max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:max_files] * 5.kilobytes = 5000.kilobytes
```
No more files are rendered at all if 5 megabytes have already been rendered.
All collection limit parameters are sent and applied on Gitaly. That is, after the limit is surpassed,
Gitaly only returns the safe amount of data to be persisted on `merge_request_diff_files`.
### Individual diff file limits
Limits that act onto each diff file of a collection. Files number, lines number and files size are considered.
#### Expandable patches (collapsed)
Diff patches are collapsed when surpassing 10% of the value set in `ApplicationSettings#diff_max_patch_bytes`.
That is, it's equivalent to 10kb if the maximum allowed value is 100kb.
The diff is persisted and expandable if the patch size doesn't
surpass `ApplicationSettings#diff_max_patch_bytes`.
Although this nomenclature (Collapsing) is also used on Gitaly, this limit is only used on GitLab (hardcoded - not sent to Gitaly).
Gitaly only returns `Diff.Collapsed` (RPC) when surpassing collection limits.
#### Not expandable patches (too large)
The patch not be rendered if it's larger than `ApplicationSettings#diff_max_patch_bytes`.
Users see a `Changes are too large to be shown.` message and a button to view only that file in that commit.
```ruby
Commit::DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000
```
File diff is suppressed (technically different from collapsed, but behaves the same, and is expandable) if it has more than 5000 lines.
This limit is hardcoded and only applied on GitLab.
## Viewers
Diff Viewers, which can be found on `models/diff_viewer/*` are classes used to map metadata about each type of Diff File. It has information
whether it's a binary, which partial should be used to render it or which File extensions this class accounts for.
`DiffViewer::Base` validates _blobs_ (old and new versions) content, extension and file type to check if it can be rendered.
## Merge request diffs against the `HEAD` of the target branch
Historically, merge request diffs have been calculated by `git diff target...source` which compares the
`HEAD` of the source branch with the merge base (or a common ancestor) of the target branch and the source's.
This solution works well until the target branch starts containing some of the
changes introduced by the source branch: Consider the following case, in which the source branch
is `feature_a` and the target is `main`:
1. Checkout a new branch `feature_a` from `main` and remove `file_a` and `file_b` in it.
1. Add a commit that removes `file_a` to `main`.
The merge request diff still contains the `file_a` removal while the actual diff compared to
`main`'s `HEAD` has only the `file_b` removal. The diff with such redundant
changes is harder to review.
In order to display an up-to-date diff, in GitLab 12.9 we
[introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/27008) merge request
diffs compared against `HEAD` of the target branch: the
target branch is artificially merged into the source branch, then the resulting
merge ref is compared to the source branch to calculate an accurate
diff.
Until we complete the epics ["use merge refs for diffs"](https://gitlab.com/groups/gitlab-org/-/epics/854)
and ["merge conflicts in diffs"](https://gitlab.com/groups/gitlab-org/-/epics/4893),
both options `main (base)` and `main (HEAD)` are available to be displayed in merge requests:
![Merge ref head options](../img/merge_ref_head_options_v13_6.png)
The `main (HEAD)` option is meant to replace `main (base)` in the future.
In order to support comments for both options, diff note positions are stored for
both `main (base)` and `main (HEAD)` versions ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/198457) in 12.10).
The position for `main (base)` version is stored in `Note#position` and
`Note#original_position` columns, for `main (HEAD)` version `DiffNotePosition`
has been introduced.
One of the key challenges to deal with when working on merge ref diffs are merge
conflicts. If the target and source branch contains a merge conflict, the branches
cannot be automatically merged. The
<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [recording on YouTube](https://www.youtube.com/watch?v=GFXIFA4ZuZw&feature=youtu.be&ab_channel=GitLabUnfiltered)
is a quick introduction to the problem and the motivation behind the [epic](https://gitlab.com/groups/gitlab-org/-/epics/854).
In 13.5 a solution for both-modified merge
conflict has been
[introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/232484). However,
there are more classes of merge conflicts that are to be
[addressed](https://gitlab.com/groups/gitlab-org/-/epics/4893) in the future.

0
doc/development/img/merge_ref_head_options_v13_6.png → doc/development/merge_request_concepts/img/merge_ref_head_options_v13_6.png
View File

Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 21 KiB

After

Width:  |  Height:  |  Size: 21 KiB

565
doc/development/merge_request_concepts/performance.md Normal file
View File

@ -0,0 +1,565 @@
---
stage: none
group: unassigned
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
---
# Merge Request Performance Guidelines
Each new introduced merge request **should be performant by default**.
To ensure a merge request does not negatively impact performance of GitLab
_every_ merge request **should** adhere to the guidelines outlined in this
document. There are no exceptions to this rule unless specifically discussed
with and agreed upon by backend maintainers and performance specialists.
It's also highly recommended that you read the following guides:
- [Performance Guidelines../performance.md)
- [Avoiding downtime in migrations](../database/avoiding_downtime_in_migrations.md)
## Definition
The term `SHOULD` per the [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt) means:
> This word, or the adjective "RECOMMENDED", mean that there
> may exist valid reasons in particular circumstances to ignore a
> particular item, but the full implications must be understood and
> carefully weighed before choosing a different course.
Ideally, each of these tradeoffs should be documented
in the separate issues, labeled accordingly and linked
to original issue and epic.
## Impact Analysis
**Summary:** think about the impact your merge request may have on performance
and those maintaining a GitLab setup.
Any change submitted can have an impact not only on the application itself but
also those maintaining it and those keeping it up and running (for example, production
engineers). As a result you should think carefully about the impact of your
merge request on not only the application but also on the people keeping it up
and running.
Can the queries used potentially take down any critical services and result in
engineers being woken up in the night? Can a malicious user abuse the code to
take down a GitLab instance? Do my changes make loading a certain page
slower? Does execution time grow exponentially given enough load or data in the
database?
These are all questions one should ask themselves before submitting a merge
request. It may sometimes be difficult to assess the impact, in which case you
should ask a performance specialist to review your code. See the "Reviewing"
section below for more information.
## Performance Review
**Summary:** ask performance specialists to review your code if you're not sure
about the impact.
Sometimes it's hard to assess the impact of a merge request. In this case you
should ask one of the merge request reviewers to review your changes. You can
find a list of these reviewers at <https://about.gitlab.com/company/team/>. A reviewer
in turn can request a performance specialist to review the changes.
## Think outside of the box
Everyone has their own perception of how to use the new feature.
Always consider how users might be using the feature instead. Usually,
users test our features in a very unconventional way,
like by brute forcing or abusing edge conditions that we have.
## Data set
The data set the merge request processes should be known
and documented. The feature should clearly document what the expected
data set is for this feature to process, and what problems it might cause.
If you would think about the following example that puts
a strong emphasis of data set being processed.
The problem is simple: you want to filter a list of files from
some Git repository. Your feature requests a list of all files
from the repository and perform search for the set of files.
As an author you should in context of that problem consider
the following:
1. What repositories are planned to be supported?
1. How long it do big repositories like Linux kernel take?
1. Is there something that we can do differently to not process such a
big data set?
1. Should we build some fail-safe mechanism to contain
computational complexity? Usually it's better to degrade
the service for a single user instead of all users.
## Query plans and database structure
The query plan can tell us if we need additional
indexes, or expensive filtering (such as using sequential scans).
Each query plan should be run against substantial size of data set.
For example, if you look for issues with specific conditions,
you should consider validating a query against
a small number (a few hundred) and a big number (100_000) of issues.
See how the query behaves if the result is a few
and a few thousand.
This is needed as we have users using GitLab for very big projects and
in a very unconventional way. Even if it seems that it's unlikely
that such a big data set is used, it's still plausible that one
of our customers could encounter a problem with the feature.
Understanding ahead of time how it behaves at scale, even if we accept it,
is the desired outcome. We should always have a plan or understanding of what is needed
to optimize the feature for higher usage patterns.
Every database structure should be optimized and sometimes even over-described
in preparation for easy extension. The hardest part after some point is
data migration. Migrating millions of rows is always troublesome and
can have a negative impact on the application.
To better understand how to get help with the query plan reviews
read this section on [how to prepare the merge request for a database review../database_review.md#how-to-prepare-the-merge-request-for-a-database-review).
## Query Counts
**Summary:** a merge request **should not** increase the total number of executed SQL
queries unless absolutely necessary.
The total number of queries executed by the code modified or added by a merge request
must not increase unless absolutely necessary. When building features it's
entirely possible you need some extra queries, but you should try to keep
this at a minimum.
As an example, say you introduce a feature that updates a number of database
rows with the same value. It may be very tempting (and easy) to write this using
the following pseudo code:
```ruby
objects_to_update.each do |object|
object.some_field = some_value
object.save
end
```
This means running one query for every object to update. This code can
easily overload a database given enough rows to update or many instances of this
code running in parallel. This particular problem is known as the
["N+1 query problem"](https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations). You can write a test with [QueryRecorder](../database/query_recorder.md) to detect this and prevent regressions.
In this particular case the workaround is fairly easy:
```ruby
objects_to_update.update_all(some_field: some_value)
```
This uses ActiveRecord's `update_all` method to update all rows in a single
query. This in turn makes it much harder for this code to overload a database.
## Use read replicas when possible
In a DB cluster we have many read replicas and one primary. A classic use of scaling the DB is to have read-only actions be performed by the replicas. We use [load balancing](../../administration/postgresql/database_load_balancing.md) to distribute this load. This allows for the replicas to grow as the pressure on the DB grows.
By default, queries use read-only replicas, but due to
[primary sticking](../../administration/postgresql/database_load_balancing.md#primary-sticking), GitLab uses the
primary for some time and reverts to secondaries after they have either caught up or after 30 seconds.
Doing this can lead to a considerable amount of unnecessary load on the primary.
To prevent switching to the primary [merge request 56849](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56849) introduced the
`without_sticky_writes` block. Typically, this method can be applied to prevent primary stickiness
after a trivial or insignificant write which doesn't affect the following queries in the same session.
To learn when a usage timestamp update can lead the session to stick to the primary and how to
prevent it by using `without_sticky_writes`, see [merge request 57328](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57328)
As a counterpart of the `without_sticky_writes` utility,
[merge request 59167](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59167) introduced
`use_replicas_for_read_queries`. This method forces all read-only queries inside its block to read
replicas regardless of the current primary stickiness.
This utility is reserved for cases where queries can tolerate replication lag.
Internally, our database load balancer classifies the queries based on their main statement (`select`, `update`, `delete`, and so on). When in doubt, it redirects the queries to the primary database. Hence, there are some common cases the load balancer sends the queries to the primary unnecessarily:
- Custom queries (via `exec_query`, `execute_statement`, `execute`, and so on)
- Read-only transactions
- In-flight connection configuration set
- Sidekiq background jobs
After the above queries are executed, GitLab
[sticks to the primary](../../administration/postgresql/database_load_balancing.md#primary-sticking).
To make the inside queries prefer using the replicas,
[merge request 59086](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59086) introduced
`fallback_to_replicas_for_ambiguous_queries`. This MR is also an example of how we redirected a
costly, time-consuming query to the replicas.
## Use CTEs wisely
Read about [complex queries on the relation object../database/iterating_tables_in_batches.md#complex-queries-on-the-relation-object) for considerations on how to use CTEs. We have found in some situations that CTEs can become problematic in use (similar to the n+1 problem above). In particular, hierarchical recursive CTE queries such as the CTE in [AuthorizedProjectsWorker](https://gitlab.com/gitlab-org/gitlab/-/issues/325688) are very difficult to optimize and don't scale. We should avoid them when implementing new features that require any kind of hierarchical structure.
CTEs have been effectively used as an optimization fence in many simpler cases,
such as this [example](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/43242#note_61416277).
Beginning in PostgreSQL 12, CTEs are inlined then [optimized by default](https://paquier.xyz/postgresql-2/postgres-12-with-materialize/).
Keeping the old behavior requires marking CTEs with the keyword `MATERIALIZED`.
When building CTE statements, use the `Gitlab::SQL::CTE` class [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56976) in GitLab 13.11.
By default, this `Gitlab::SQL::CTE` class forces materialization through adding the `MATERIALIZED` keyword for PostgreSQL 12 and higher.
`Gitlab::SQL::CTE` automatically omits materialization when PostgreSQL 11 is running
(this behavior is implemented using a custom Arel node `Gitlab::Database::AsWithMaterialized` under the surface).
WARNING:
Upgrading to GitLab 14.0 requires PostgreSQL 12 or higher.
## Cached Queries
**Summary:** a merge request **should not** execute duplicated cached queries.
Rails provides an [SQL Query Cache](../cached_queries.md#cached-queries-guidelines),
used to cache the results of database queries for the duration of the request.
See [why cached queries are considered bad](../cached_queries.md#why-cached-queries-are-considered-bad) and
[how to detect them](../cached_queries.md#how-to-detect-cached-queries).
The code introduced by a merge request, should not execute multiple duplicated cached queries.
The total number of the queries (including cached ones) executed by the code modified or added by a merge request
should not increase unless absolutely necessary.
The number of executed queries (including cached queries) should not depend on
collection size.
You can write a test by passing the `skip_cached` variable to [QueryRecorder../database/query_recorder.md) to detect this and prevent regressions.
As an example, say you have a CI pipeline. All pipeline builds belong to the same pipeline,
thus they also belong to the same project (`pipeline.project`):
```ruby
pipeline_project = pipeline.project
# Project Load (0.6ms) SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 LIMIT $2
build = pipeline.builds.first
build.project == pipeline_project
# CACHE Project Load (0.0ms) SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 LIMIT $2
# => true
```
When we call `build.project`, it doesn't hit the database, it uses the cached result, but it re-instantiates
the same pipeline project object. It turns out that associated objects do not point to the same in-memory object.
If we try to serialize each build:
```ruby
pipeline.builds.each do |build|
build.to_json(only: [:name], include: [project: { only: [:name]}])
end
```
It re-instantiates project object for each build, instead of using the same in-memory object.
In this particular case the workaround is fairly easy:
```ruby
ActiveRecord::Associations::Preloader.new.preload(pipeline, [builds: :project])
pipeline.builds.each do |build|
build.to_json(only: [:name], include: [project: { only: [:name]}])
end
```
`ActiveRecord::Associations::Preloader` uses the same in-memory object for the same project.
This avoids the cached SQL query and also avoids re-instantiation of the project object for each build.
## Executing Queries in Loops
**Summary:** SQL queries **must not** be executed in a loop unless absolutely
necessary.
Executing SQL queries in a loop can result in many queries being executed
depending on the number of iterations in a loop. This may work fine for a
development environment with little data, but in a production environment this
can quickly spiral out of control.
There are some cases where this may be needed. If this is the case this should
be clearly mentioned in the merge request description.
## Batch process
**Summary:** Iterating a single process to external services (for example, PostgreSQL, Redis, Object Storage)
should be executed in a **batch-style** to reduce connection overheads.
For fetching rows from various tables in a batch-style, please see [Eager Loading](#eager-loading) section.
### Example: Delete multiple files from Object Storage
When you delete multiple files from object storage, like GCS,
executing a single REST API call multiple times is a quite expensive
process. Ideally, this should be done in a batch-style, for example, S3 provides
[batch deletion API](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html),
so it'd be a good idea to consider such an approach.
The `FastDestroyAll` module might help this situation. It's a
small framework when you remove a bunch of database rows and its associated data
in a batch style.
## Timeout
**Summary:** You should set a reasonable timeout when the system invokes HTTP calls
to external services (such as Kubernetes), and it should be executed in Sidekiq, not
in Puma threads.
Often, GitLab needs to communicate with an external service such as Kubernetes
clusters. In this case, it's hard to estimate when the external service finishes
the requested process, for example, if it's a user-owned cluster that's inactive for some reason,
GitLab might wait for the response forever ([Example](https://gitlab.com/gitlab-org/gitlab/-/issues/31475)).
This could result in Puma timeout and should be avoided at all cost.
You should set a reasonable timeout, gracefully handle exceptions and surface the
errors in UI or logging internally.
Using [`ReactiveCaching`../utilities.md#reactivecaching) is one of the best solutions to fetch external data.
## Keep database transaction minimal
**Summary:** You should avoid accessing to external services like Gitaly during database
transactions, otherwise it leads to severe contention problems
as an open transaction basically blocks the release of a PostgreSQL backend connection.
For keeping transaction as minimal as possible, please consider using `AfterCommitQueue`
module or `after_commit` AR hook.
Here is [an example](https://gitlab.com/gitlab-org/gitlab/-/issues/36154#note_247228859)
that one request to Gitaly instance during transaction triggered a ~"priority::1" issue.
## Eager Loading
**Summary:** always eager load associations when retrieving more than one row.
When retrieving multiple database records for which you need to use any
associations you **must** eager load these associations. For example, if you're
retrieving a list of blog posts and you want to display their authors you
**must** eager load the author associations.
In other words, instead of this:
```ruby
Post.all.each do |post|
puts post.author.name
end
```
You should use this:
```ruby
Post.all.includes(:author).each do |post|
puts post.author.name
end
```
Also consider using [QueryRecoder tests](../database/query_recorder.md) to prevent a regression when eager loading.
## Memory Usage
**Summary:** merge requests **must not** increase memory usage unless absolutely
necessary.
A merge request must not increase the memory usage of GitLab by more than the
absolute bare minimum required by the code. This means that if you have to parse
some large document (for example, an HTML document) it's best to parse it as a stream
whenever possible, instead of loading the entire input into memory. Sometimes
this isn't possible, in that case this should be stated explicitly in the merge
request.
## Lazy Rendering of UI Elements
**Summary:** only render UI elements when they are actually needed.
Certain UI elements may not always be needed. For example, when hovering over a
diff line there's a small icon displayed that can be used to create a new
comment. Instead of always rendering these kind of elements they should only be
rendered when actually needed. This ensures we don't spend time generating
Haml/HTML when it's not used.
## Use of Caching
**Summary:** cache data in memory or in Redis when it's needed multiple times in
a transaction or has to be kept around for a certain time period.
Sometimes certain bits of data have to be re-used in different places during a
transaction. In these cases this data should be cached in memory to remove the
need for running complex operations to fetch the data. You should use Redis if
data should be cached for a certain time period instead of the duration of the
transaction.
For example, say you process multiple snippets of text containing username
mentions (for example, `Hello @alice` and `How are you doing @alice?`). By caching the
user objects for every username we can remove the need for running the same
query for every mention of `@alice`.
Caching data per transaction can be done using
[RequestStore](https://github.com/steveklabnik/request_store) (use
`Gitlab::SafeRequestStore` to avoid having to remember to check
`RequestStore.active?`). Caching data in Redis can be done using
[Rails' caching system](https://guides.rubyonrails.org/caching_with_rails.html).
## Pagination
Each feature that renders a list of items as a table needs to include pagination.
The main styles of pagination are:
1. Offset-based pagination: user goes to a specific page, like 1. User sees the next page number,
and the total number of pages. This style is well supported by all components of GitLab.
1. Offset-based pagination, but without the count: user goes to a specific page, like 1.
User sees only the next page number, but does not see the total amount of pages.
1. Next page using keyset-based pagination: user can only go to next page, as we don't know how many pages
are available.
1. Infinite scrolling pagination: user scrolls the page and next items are loaded asynchronously. This is ideal,
as it has exact same benefits as the previous one.
The ultimately scalable solution for pagination is to use Keyset-based pagination.
However, we don't have support for that at GitLab at that moment. You
can follow the progress looking at [API: Keyset Pagination](https://gitlab.com/groups/gitlab-org/-/epics/2039).
Take into consideration the following when choosing a pagination strategy:
1. It's very inefficient to calculate amount of objects that pass the filtering,
this operation usually can take seconds, and can time out,
1. It's very inefficient to get entries for page at higher ordinals, like 1000.
The database has to sort and iterate all previous items, and this operation usually
can result in substantial load put on database.
You can find useful tips related to pagination in the [pagination guidelines../database/pagination_guidelines.md).
## Badge counters
Counters should always be truncated. It means that we don't want to present
the exact number over some threshold. The reason for that is for the cases where we want
to calculate exact number of items, we effectively need to filter each of them for
the purpose of knowing the exact number of items matching.
From ~UX perspective it's often acceptable to see that you have over 1000+ pipelines,
instead of that you have 40000+ pipelines, but at a tradeoff of loading page for 2s longer.
An example of this pattern is the list of pipelines and jobs. We truncate numbers to `1000+`,
but we show an accurate number of running pipelines, which is the most interesting information.
There's a helper method that can be used for that purpose - `NumbersHelper.limited_counter_with_delimiter` -
that accepts an upper limit of counting rows.
In some cases it's desired that badge counters are loaded asynchronously.
This can speed up the initial page load and give a better user experience overall.
## Usage of feature flags
Each feature that has performance critical elements or has a known performance deficiency
needs to come with feature flag to disable it.
The feature flag makes our team more happy, because they can monitor the system and
quickly react without our users noticing the problem.
Performance deficiencies should be addressed right away after we merge initial
changes.
Read more about when and how feature flags should be used in
[Feature flags in GitLab development](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle/#feature-flags-in-gitlab-development).
## Storage
We can consider the following types of storages:
- **Local temporary storage** (very-very short-term storage) This type of storage is system-provided storage, like a `/tmp` folder.
This is the type of storage that you should ideally use for all your temporary tasks.
The fact that each node has its own temporary storage makes scaling significantly easier.
This storage is also very often SSD-based, thus is significantly faster.
The local storage can easily be configured for the application with
the usage of `TMPDIR` variable.
- **Shared temporary storage** (short-term storage) This type of storage is network-based temporary storage,
usually run with a common NFS server. As of Feb 2020, we still use this type of storage
for most of our implementations. Even though this allows the above limit to be significantly larger,
it does not really mean that you can use more. The shared temporary storage is shared by
all nodes. Thus, the job that uses significant amount of that space or performs a lot
of operations creates a contention on execution of all other jobs and request
across the whole application, this can easily impact stability of the whole GitLab.
Be respectful of that.
- **Shared persistent storage** (long-term storage) This type of storage uses
shared network-based storage (for example, NFS). This solution is mostly used by customers running small
installations consisting of a few nodes. The files on shared storage are easily accessible,
but any job that is uploading or downloading data can create a serious contention to all other jobs.
This is also an approach by default used by Omnibus.
- **Object-based persistent storage** (long term storage) this type of storage uses external
services like [AWS S3](https://en.wikipedia.org/wiki/Amazon_S3). The Object Storage
can be treated as infinitely scalable and redundant. Accessing this storage usually requires
downloading the file to manipulate it. The Object Storage can be considered as an ultimate
solution, as by definition it can be assumed that it can handle unlimited concurrent uploads
and downloads of files. This is also ultimate solution required to ensure that application can
run in containerized deployments (Kubernetes) at ease.
### Temporary storage
The storage on production nodes is really sparse. The application should be built
in a way that accommodates running under very limited temporary storage.
You can expect the system on which your code runs has a total of `1G-10G`
of temporary storage. However, this storage is really shared across all
jobs being run. If your job requires to use more than `100MB` of that space
you should reconsider the approach you have taken.
Whatever your needs are, you should clearly document if you need to process files.
If you require more than `100MB`, consider asking for help from a maintainer
to work with you to possibly discover a better solution.
#### Local temporary storage
The usage of local storage is a desired solution to use,
especially since we work on deploying applications to Kubernetes clusters.
When you would like to use `Dir.mktmpdir`? In a case when you want for example
to extract/create archives, perform extensive manipulation of existing data, and so on.
```ruby
Dir.mktmpdir('designs') do |path|
# do manipulation on path
# the path will be removed once
# we go out of the block
end
```
#### Shared temporary storage
The usage of shared temporary storage is required if your intent
is to persistent file for a disk-based storage, and not Object Storage.
[Workhorse direct upload](../uploads/index.md#direct-upload) when accepting file
can write it to shared storage, and later GitLab Rails can perform a move operation.
The move operation on the same destination is instantaneous.
The system instead of performing `copy` operation just re-attaches file into a new place.
Since this introduces extra complexity into application, you should only try
to re-use well established patterns (for example, `ObjectStorage` concern) instead of re-implementing it.
The usage of shared temporary storage is otherwise deprecated for all other usages.
### Persistent storage
#### Object Storage
It is required that all features holding persistent files support saving data
to Object Storage. Having a persistent storage in the form of shared volume across nodes
is not scalable, as it creates a contention on data access all nodes.
GitLab offers the [ObjectStorage concern](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/uploaders/object_storage.rb)
that implements a seamless support for Shared and Object Storage-based persistent storage.
#### Data access
Each feature that accepts data uploads or allows to download them needs to use
[Workhorse direct upload](../uploads/index.md#direct-upload). It means that uploads needs to be
saved directly to Object Storage by Workhorse, and all downloads needs to be served
by Workhorse.
Performing uploads/downloads via Puma is an expensive operation,
as it blocks the whole processing slot (thread) for the duration of the upload.
Performing uploads/downloads via Puma also has a problem where the operation
can time out, which is especially problematic for slow clients. If clients take a long time
to upload/download the processing slot might be killed due to request processing
timeout (usually between 30s-60s).
For the above reasons it is required that [Workhorse direct upload../uploads/index.md#direct-upload) is implemented
for all file uploads and downloads.

191
doc/development/merge_request_diffs.md
View File

@ -1,188 +1,11 @@
---
stage: Create
group: Code Review
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
redirect_to: 'merge_request_concepts/diffs/development.md'
remove_date: '2023-04-10'
---
# Merge request diffs development guide **(FREE)**
This document was moved to [another location](merge_request_concepts/diffs/development.md).
This document explains the backend design and flow of merge request diffs.
It should help contributors:
- Understand the code design.
- Identify areas for improvement through contribution.
It's intentional that it doesn't contain too many implementation details, as they
can change often. The code better explains these details. The components
mentioned here are the major parts of the application for how merge request diffs
are generated, stored, and returned to users.
NOTE:
This page is a living document. Update it accordingly when the parts
of the codebase touched in this document are changed or removed, or when new components
are added.
## Data model
Four main ActiveRecord models represent what we collectively refer to
as _diffs._ These database-backed records replicate data contained in the
project's Git repository, and are in part a cache against excessive access requests
to [Gitaly](gitaly.md). Additionally, they provide a logical place for:
- Calculated and retrieved metadata about the pieces of the diff.
- General class- and instance- based logic.
```mermaid
erDiagram
MergeRequest ||--|{ MergeRequestDiff: ""
MergeRequestDiff |{--|{ MergeRequestDiffCommit: ""
MergeRequestDiff |{--|| MergeRequestDiffDetail: ""
MergeRequestDiff |{--|{ MergeRequestDiffFile: ""
MergeRequestDiffCommit |{--|| MergeRequestDiffCommitUser: ""
```
### `MergeRequestDiff`
`MergeRequestDiff` is defined in `app/models/merge_request_diff.rb`. This
class holds metadata and context related to the diff resulting from a set of
commits. It defines methods that are the primary means for interacting with diff
contents, individual commits, and the files containing changes.
```ruby
#<MergeRequestDiff:0x00007fd1ed63b4d0
id: 28,
state: "collected",
merge_request_id: 28,
created_at: Tue, 06 Sep 2022 18:56:02.509469000 UTC +00:00,
updated_at: Tue, 06 Sep 2022 18:56:02.754201000 UTC +00:00,
base_commit_sha: "ae73cb07c9eeaf35924a10f713b364d32b2dd34f",
real_size: "9",
head_commit_sha: "bb5206fee213d983da88c47f9cf4cc6caf9c66dc",
start_commit_sha: "0b4bc9a49b562e85de7cc9e834518ea6828729b9",
commits_count: 6,
external_diff: "diff-28",
external_diff_store: 1,
stored_externally: nil,
files_count: 9,
sorted: true,
diff_type: "regular",
verification_checksum: nil>
```
Diff content is usually accessed through this class. Logic is often applied
to diff, file, and commit content before it is returned to a user.
### `MergeRequestDiffCommit`
`MergeRequestDiffCommit` is defined in `app/models/merge_request_diff_commit.rb`.
This class corresponds to a single commit contained in its corresponding `MergeRequestDiff`,
and holds header information about the commit.
```ruby
#<MergeRequestDiffCommit:0x00007fd1dfc6c4c0
authored_date: Wed, 06 Aug 2022 06:35:52.000000000 UTC +00:00,
committed_date: Wed, 06 Aug 2022 06:35:52.000000000 UTC +00:00,
merge_request_diff_id: 28,
relative_order: 0,
sha: "bb5206fee213d983da88c47f9cf4cc6caf9c66dc",
message: "Feature conflcit added\n\nSigned-off-by: Sample User <sample.user@example.com>\n",
trailers: {},
commit_author_id: 19,
committer_id: 19>
```
Every `MergeRequestDiffCommit` has a corresponding `MergeRequest::DiffCommitUser`
record it `:belongs_to`, in ActiveRecord parlance. These records are `:commit_author`
and `:committer`, and could be distinct individuals.
### `MergeRequest::DiffCommitUser`
`MergeRequest::DiffCommitUser` is defined in `app/models/merge_request/diff_commit_user.rb`.
It captures the `name` and `email` of a given commit, but contains no connection
itself to any `User` records.
```ruby
#<MergeRequest::DiffCommitUser:0x00007fd1dff7c930
id: 19,
name: "Sample User",
email: "sample.user@example.com">
```
### `MergeRequestDiffFile`
`MergeRequestDiffFile` is defined in `app/models/merge_request_diff_file.rb`.
This record of this class represents the diff of a single file contained in the
`MergeRequestDiff`. It holds both meta and specific information about the file's
relationship to the change, such as:
- Whether it is added or renamed.
- Its ordering in the diff.
- The raw diff output itself.
### `MergeRequestDiffDetail`
`MergeRequestDiffDetail` is defined in `app/models/merge_request_diff_detail.rb`.
This class provides verification information for Geo replication, but otherwise
is not used for user-facing diffs.
```ruby
#<MergeRequestDiffFile:0x00007fd1ef7c9048
merge_request_diff_id: 28,
relative_order: 0,
new_file: true,
renamed_file: false,
deleted_file: false,
too_large: false,
a_mode: "0",
b_mode: "100644",
new_path: "files/ruby/feature.rb",
old_path: "files/ruby/feature.rb",
diff:
"@@ -0,0 +1,4 @@\n+# This file was changed in feature branch\n+# We put different code here to make merge conflict\n+class Conflict\n+end\n",
binary: false,
external_diff_offset: nil,
external_diff_size: nil>
```
## Flow
These flowcharts should help explain the flow from the controllers down to the
models for different features. This page is not intended to document the entirety
of options for access and working with diffs, focusing solely on the most common.
### `batch_diffs.json`
The most common avenue for viewing diffs is the **Changes**
tab in the top navigation bar of merge request pages in the GitLab UI. When selected, the
diffs themselves are loaded via a paginated request to `/-/merge_requests/:id/batch_diffs.json`,
which is served by [`Projects::MergeRequests::DiffsController#diffs_batch`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/controllers/projects/merge_requests/diffs_controller.rb):
<!-- Don't delete the &nbsp; characters below. Mermaid returns a syntax error if they aren't included.-->
```mermaid
sequenceDiagram
Note over .#diffs_batch: Preload diffs and ivars
.#diffs_batch->>+.#define_diff_vars: &nbsp;
.#define_diff_vars ->>+ @merge_request: @merge_request_diffs =
Note right of @merge_request: An ordered collection of all diffs in MR
@merge_request-->>-.#define_diff_vars: &nbsp;
.#define_diff_vars ->>+ @merge_request: @merge_request_diff =
Note right of @merge_request: Most recent merge_request_diff (or commit)
@merge_request-->>-.#define_diff_vars: &nbsp;
.#define_diff_vars ->>+ .#define_diff_vars: @compare =
Note right of .#define_diff_vars:: param-filtered merge_request_diff(s)
.#define_diff_vars -->>- .#diffs_batch: &nbsp;
Note over .#diffs_batch: Preloading complete
.#diffs_batch->>+@merge_request: Calculate unfoldable diff lines
Note right of @merge_request: note_positions_for_paths.unfoldable
@merge_request-->>-.#diffs_batch: &nbsp;
Note over .#diffs_batch: Build options hash
Note over .#diffs_batch: Build cache_context
Note over .#diffs_batch: Unfold files in diff
.#diffs_batch->>+Gitlab_Diff_FileCollection_MergeRequestDiffBase: diffs.write_diff
Gitlab_Diff_FileCollection_MergeRequestDiffBase->>+Gitlab_Diff_HighlightCache: Highlight diff
Gitlab_Diff_HighlightCache -->>-Gitlab_Diff_FileCollection_MergeRequestDiffBase: Return highlighted diff
Note over Gitlab_Diff_FileCollection_MergeRequestDiffBase: Cache diff
Gitlab_Diff_FileCollection_MergeRequestDiffBase-->>-.#diffs_batch: &nbsp;
Note over .#diffs_batch: render JSON
```
<!-- This redirect file can be deleted after <2023-04-10>. -->
<!-- Redirects that point to other docs in the same project expire in three months. -->
<!-- Redirects that point to docs in a different project or site (for example, link is not relative and starts with `https:`) expire in one year. -->
<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/redirects.html -->

568
doc/development/merge_request_performance_guidelines.md
View File

@ -1,565 +1,11 @@
---
stage: none
group: unassigned
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
redirect_to: 'merge_request_concepts/performance.md'
remove_date: '2023-04-10'
---
# Merge Request Performance Guidelines
This document was moved to [another location](merge_request_concepts/performance.md).
Each new introduced merge request **should be performant by default**.
To ensure a merge request does not negatively impact performance of GitLab
_every_ merge request **should** adhere to the guidelines outlined in this
document. There are no exceptions to this rule unless specifically discussed
with and agreed upon by backend maintainers and performance specialists.
It's also highly recommended that you read the following guides:
- [Performance Guidelines](performance.md)
- [Avoiding downtime in migrations](database/avoiding_downtime_in_migrations.md)
## Definition
The term `SHOULD` per the [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt) means:
> This word, or the adjective "RECOMMENDED", mean that there
> may exist valid reasons in particular circumstances to ignore a
> particular item, but the full implications must be understood and
> carefully weighed before choosing a different course.
Ideally, each of these tradeoffs should be documented
in the separate issues, labeled accordingly and linked
to original issue and epic.
## Impact Analysis
**Summary:** think about the impact your merge request may have on performance
and those maintaining a GitLab setup.
Any change submitted can have an impact not only on the application itself but
also those maintaining it and those keeping it up and running (for example, production
engineers). As a result you should think carefully about the impact of your
merge request on not only the application but also on the people keeping it up
and running.
Can the queries used potentially take down any critical services and result in
engineers being woken up in the night? Can a malicious user abuse the code to
take down a GitLab instance? Do my changes make loading a certain page
slower? Does execution time grow exponentially given enough load or data in the
database?
These are all questions one should ask themselves before submitting a merge
request. It may sometimes be difficult to assess the impact, in which case you
should ask a performance specialist to review your code. See the "Reviewing"
section below for more information.
## Performance Review
**Summary:** ask performance specialists to review your code if you're not sure
about the impact.
Sometimes it's hard to assess the impact of a merge request. In this case you
should ask one of the merge request reviewers to review your changes. You can
find a list of these reviewers at <https://about.gitlab.com/company/team/>. A reviewer
in turn can request a performance specialist to review the changes.
## Think outside of the box
Everyone has their own perception of how to use the new feature.
Always consider how users might be using the feature instead. Usually,
users test our features in a very unconventional way,
like by brute forcing or abusing edge conditions that we have.
## Data set
The data set the merge request processes should be known
and documented. The feature should clearly document what the expected
data set is for this feature to process, and what problems it might cause.
If you would think about the following example that puts
a strong emphasis of data set being processed.
The problem is simple: you want to filter a list of files from
some Git repository. Your feature requests a list of all files
from the repository and perform search for the set of files.
As an author you should in context of that problem consider
the following:
1. What repositories are planned to be supported?
1. How long it do big repositories like Linux kernel take?
1. Is there something that we can do differently to not process such a
big data set?
1. Should we build some fail-safe mechanism to contain
computational complexity? Usually it's better to degrade
the service for a single user instead of all users.
## Query plans and database structure
The query plan can tell us if we need additional
indexes, or expensive filtering (such as using sequential scans).
Each query plan should be run against substantial size of data set.
For example, if you look for issues with specific conditions,
you should consider validating a query against
a small number (a few hundred) and a big number (100_000) of issues.
See how the query behaves if the result is a few
and a few thousand.
This is needed as we have users using GitLab for very big projects and
in a very unconventional way. Even if it seems that it's unlikely
that such a big data set is used, it's still plausible that one
of our customers could encounter a problem with the feature.
Understanding ahead of time how it behaves at scale, even if we accept it,
is the desired outcome. We should always have a plan or understanding of what is needed
to optimize the feature for higher usage patterns.
Every database structure should be optimized and sometimes even over-described
in preparation for easy extension. The hardest part after some point is
data migration. Migrating millions of rows is always troublesome and
can have a negative impact on the application.
To better understand how to get help with the query plan reviews
read this section on [how to prepare the merge request for a database review](database_review.md#how-to-prepare-the-merge-request-for-a-database-review).
## Query Counts
**Summary:** a merge request **should not** increase the total number of executed SQL
queries unless absolutely necessary.
The total number of queries executed by the code modified or added by a merge request
must not increase unless absolutely necessary. When building features it's
entirely possible you need some extra queries, but you should try to keep
this at a minimum.
As an example, say you introduce a feature that updates a number of database
rows with the same value. It may be very tempting (and easy) to write this using
the following pseudo code:
```ruby
objects_to_update.each do |object|
object.some_field = some_value
object.save
end
```
This means running one query for every object to update. This code can
easily overload a database given enough rows to update or many instances of this
code running in parallel. This particular problem is known as the
["N+1 query problem"](https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations). You can write a test with [QueryRecorder](database/query_recorder.md) to detect this and prevent regressions.
In this particular case the workaround is fairly easy:
```ruby
objects_to_update.update_all(some_field: some_value)
```
This uses ActiveRecord's `update_all` method to update all rows in a single
query. This in turn makes it much harder for this code to overload a database.
## Use read replicas when possible
In a DB cluster we have many read replicas and one primary. A classic use of scaling the DB is to have read-only actions be performed by the replicas. We use [load balancing](../administration/postgresql/database_load_balancing.md) to distribute this load. This allows for the replicas to grow as the pressure on the DB grows.
By default, queries use read-only replicas, but due to
[primary sticking](../administration/postgresql/database_load_balancing.md#primary-sticking), GitLab uses the
primary for some time and reverts to secondaries after they have either caught up or after 30 seconds.
Doing this can lead to a considerable amount of unnecessary load on the primary.
To prevent switching to the primary [merge request 56849](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56849) introduced the
`without_sticky_writes` block. Typically, this method can be applied to prevent primary stickiness
after a trivial or insignificant write which doesn't affect the following queries in the same session.
To learn when a usage timestamp update can lead the session to stick to the primary and how to
prevent it by using `without_sticky_writes`, see [merge request 57328](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57328)
As a counterpart of the `without_sticky_writes` utility,
[merge request 59167](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59167) introduced
`use_replicas_for_read_queries`. This method forces all read-only queries inside its block to read
replicas regardless of the current primary stickiness.
This utility is reserved for cases where queries can tolerate replication lag.
Internally, our database load balancer classifies the queries based on their main statement (`select`, `update`, `delete`, and so on). When in doubt, it redirects the queries to the primary database. Hence, there are some common cases the load balancer sends the queries to the primary unnecessarily:
- Custom queries (via `exec_query`, `execute_statement`, `execute`, and so on)
- Read-only transactions
- In-flight connection configuration set
- Sidekiq background jobs
After the above queries are executed, GitLab
[sticks to the primary](../administration/postgresql/database_load_balancing.md#primary-sticking).
To make the inside queries prefer using the replicas,
[merge request 59086](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59086) introduced
`fallback_to_replicas_for_ambiguous_queries`. This MR is also an example of how we redirected a
costly, time-consuming query to the replicas.
## Use CTEs wisely
Read about [complex queries on the relation object](database/iterating_tables_in_batches.md#complex-queries-on-the-relation-object) for considerations on how to use CTEs. We have found in some situations that CTEs can become problematic in use (similar to the n+1 problem above). In particular, hierarchical recursive CTE queries such as the CTE in [AuthorizedProjectsWorker](https://gitlab.com/gitlab-org/gitlab/-/issues/325688) are very difficult to optimize and don't scale. We should avoid them when implementing new features that require any kind of hierarchical structure.
CTEs have been effectively used as an optimization fence in many simpler cases,
such as this [example](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/43242#note_61416277).
Beginning in PostgreSQL 12, CTEs are inlined then [optimized by default](https://paquier.xyz/postgresql-2/postgres-12-with-materialize/).
Keeping the old behavior requires marking CTEs with the keyword `MATERIALIZED`.
When building CTE statements, use the `Gitlab::SQL::CTE` class [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56976) in GitLab 13.11.
By default, this `Gitlab::SQL::CTE` class forces materialization through adding the `MATERIALIZED` keyword for PostgreSQL 12 and higher.
`Gitlab::SQL::CTE` automatically omits materialization when PostgreSQL 11 is running
(this behavior is implemented using a custom Arel node `Gitlab::Database::AsWithMaterialized` under the surface).
WARNING:
Upgrading to GitLab 14.0 requires PostgreSQL 12 or higher.
## Cached Queries
**Summary:** a merge request **should not** execute duplicated cached queries.
Rails provides an [SQL Query Cache](cached_queries.md#cached-queries-guidelines),
used to cache the results of database queries for the duration of the request.
See [why cached queries are considered bad](cached_queries.md#why-cached-queries-are-considered-bad) and
[how to detect them](cached_queries.md#how-to-detect-cached-queries).
The code introduced by a merge request, should not execute multiple duplicated cached queries.
The total number of the queries (including cached ones) executed by the code modified or added by a merge request
should not increase unless absolutely necessary.
The number of executed queries (including cached queries) should not depend on
collection size.
You can write a test by passing the `skip_cached` variable to [QueryRecorder](database/query_recorder.md) to detect this and prevent regressions.
As an example, say you have a CI pipeline. All pipeline builds belong to the same pipeline,
thus they also belong to the same project (`pipeline.project`):
```ruby
pipeline_project = pipeline.project
# Project Load (0.6ms) SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 LIMIT $2
build = pipeline.builds.first
build.project == pipeline_project
# CACHE Project Load (0.0ms) SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 LIMIT $2
# => true
```
When we call `build.project`, it doesn't hit the database, it uses the cached result, but it re-instantiates
the same pipeline project object. It turns out that associated objects do not point to the same in-memory object.
If we try to serialize each build:
```ruby
pipeline.builds.each do |build|
build.to_json(only: [:name], include: [project: { only: [:name]}])
end
```
It re-instantiates project object for each build, instead of using the same in-memory object.
In this particular case the workaround is fairly easy:
```ruby
ActiveRecord::Associations::Preloader.new.preload(pipeline, [builds: :project])
pipeline.builds.each do |build|
build.to_json(only: [:name], include: [project: { only: [:name]}])
end
```
`ActiveRecord::Associations::Preloader` uses the same in-memory object for the same project.
This avoids the cached SQL query and also avoids re-instantiation of the project object for each build.
## Executing Queries in Loops
**Summary:** SQL queries **must not** be executed in a loop unless absolutely
necessary.
Executing SQL queries in a loop can result in many queries being executed
depending on the number of iterations in a loop. This may work fine for a
development environment with little data, but in a production environment this
can quickly spiral out of control.
There are some cases where this may be needed. If this is the case this should
be clearly mentioned in the merge request description.
## Batch process
**Summary:** Iterating a single process to external services (for example, PostgreSQL, Redis, Object Storage)
should be executed in a **batch-style** to reduce connection overheads.
For fetching rows from various tables in a batch-style, please see [Eager Loading](#eager-loading) section.
### Example: Delete multiple files from Object Storage
When you delete multiple files from object storage, like GCS,
executing a single REST API call multiple times is a quite expensive
process. Ideally, this should be done in a batch-style, for example, S3 provides
[batch deletion API](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html),
so it'd be a good idea to consider such an approach.
The `FastDestroyAll` module might help this situation. It's a
small framework when you remove a bunch of database rows and its associated data
in a batch style.
## Timeout
**Summary:** You should set a reasonable timeout when the system invokes HTTP calls
to external services (such as Kubernetes), and it should be executed in Sidekiq, not
in Puma threads.
Often, GitLab needs to communicate with an external service such as Kubernetes
clusters. In this case, it's hard to estimate when the external service finishes
the requested process, for example, if it's a user-owned cluster that's inactive for some reason,
GitLab might wait for the response forever ([Example](https://gitlab.com/gitlab-org/gitlab/-/issues/31475)).
This could result in Puma timeout and should be avoided at all cost.
You should set a reasonable timeout, gracefully handle exceptions and surface the
errors in UI or logging internally.
Using [`ReactiveCaching`](utilities.md#reactivecaching) is one of the best solutions to fetch external data.
## Keep database transaction minimal
**Summary:** You should avoid accessing to external services like Gitaly during database
transactions, otherwise it leads to severe contention problems
as an open transaction basically blocks the release of a PostgreSQL backend connection.
For keeping transaction as minimal as possible, please consider using `AfterCommitQueue`
module or `after_commit` AR hook.
Here is [an example](https://gitlab.com/gitlab-org/gitlab/-/issues/36154#note_247228859)
that one request to Gitaly instance during transaction triggered a ~"priority::1" issue.
## Eager Loading
**Summary:** always eager load associations when retrieving more than one row.
When retrieving multiple database records for which you need to use any
associations you **must** eager load these associations. For example, if you're
retrieving a list of blog posts and you want to display their authors you
**must** eager load the author associations.
In other words, instead of this:
```ruby
Post.all.each do |post|
puts post.author.name
end
```
You should use this:
```ruby
Post.all.includes(:author).each do |post|
puts post.author.name
end
```
Also consider using [QueryRecoder tests](database/query_recorder.md) to prevent a regression when eager loading.
## Memory Usage
**Summary:** merge requests **must not** increase memory usage unless absolutely
necessary.
A merge request must not increase the memory usage of GitLab by more than the
absolute bare minimum required by the code. This means that if you have to parse
some large document (for example, an HTML document) it's best to parse it as a stream
whenever possible, instead of loading the entire input into memory. Sometimes
this isn't possible, in that case this should be stated explicitly in the merge
request.
## Lazy Rendering of UI Elements
**Summary:** only render UI elements when they are actually needed.
Certain UI elements may not always be needed. For example, when hovering over a
diff line there's a small icon displayed that can be used to create a new
comment. Instead of always rendering these kind of elements they should only be
rendered when actually needed. This ensures we don't spend time generating
Haml/HTML when it's not used.
## Use of Caching
**Summary:** cache data in memory or in Redis when it's needed multiple times in
a transaction or has to be kept around for a certain time period.
Sometimes certain bits of data have to be re-used in different places during a
transaction. In these cases this data should be cached in memory to remove the
need for running complex operations to fetch the data. You should use Redis if
data should be cached for a certain time period instead of the duration of the
transaction.
For example, say you process multiple snippets of text containing username
mentions (for example, `Hello @alice` and `How are you doing @alice?`). By caching the
user objects for every username we can remove the need for running the same
query for every mention of `@alice`.
Caching data per transaction can be done using
[RequestStore](https://github.com/steveklabnik/request_store) (use
`Gitlab::SafeRequestStore` to avoid having to remember to check
`RequestStore.active?`). Caching data in Redis can be done using
[Rails' caching system](https://guides.rubyonrails.org/caching_with_rails.html).
## Pagination
Each feature that renders a list of items as a table needs to include pagination.
The main styles of pagination are:
1. Offset-based pagination: user goes to a specific page, like 1. User sees the next page number,
and the total number of pages. This style is well supported by all components of GitLab.
1. Offset-based pagination, but without the count: user goes to a specific page, like 1.
User sees only the next page number, but does not see the total amount of pages.
1. Next page using keyset-based pagination: user can only go to next page, as we don't know how many pages
are available.
1. Infinite scrolling pagination: user scrolls the page and next items are loaded asynchronously. This is ideal,
as it has exact same benefits as the previous one.
The ultimately scalable solution for pagination is to use Keyset-based pagination.
However, we don't have support for that at GitLab at that moment. You
can follow the progress looking at [API: Keyset Pagination](https://gitlab.com/groups/gitlab-org/-/epics/2039).
Take into consideration the following when choosing a pagination strategy:
1. It's very inefficient to calculate amount of objects that pass the filtering,
this operation usually can take seconds, and can time out,
1. It's very inefficient to get entries for page at higher ordinals, like 1000.
The database has to sort and iterate all previous items, and this operation usually
can result in substantial load put on database.
You can find useful tips related to pagination in the [pagination guidelines](database/pagination_guidelines.md).
## Badge counters
Counters should always be truncated. It means that we don't want to present
the exact number over some threshold. The reason for that is for the cases where we want
to calculate exact number of items, we effectively need to filter each of them for
the purpose of knowing the exact number of items matching.
From ~UX perspective it's often acceptable to see that you have over 1000+ pipelines,
instead of that you have 40000+ pipelines, but at a tradeoff of loading page for 2s longer.
An example of this pattern is the list of pipelines and jobs. We truncate numbers to `1000+`,
but we show an accurate number of running pipelines, which is the most interesting information.
There's a helper method that can be used for that purpose - `NumbersHelper.limited_counter_with_delimiter` -
that accepts an upper limit of counting rows.
In some cases it's desired that badge counters are loaded asynchronously.
This can speed up the initial page load and give a better user experience overall.
## Usage of feature flags
Each feature that has performance critical elements or has a known performance deficiency
needs to come with feature flag to disable it.
The feature flag makes our team more happy, because they can monitor the system and
quickly react without our users noticing the problem.
Performance deficiencies should be addressed right away after we merge initial
changes.
Read more about when and how feature flags should be used in
[Feature flags in GitLab development](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle/#feature-flags-in-gitlab-development).
## Storage
We can consider the following types of storages:
- **Local temporary storage** (very-very short-term storage) This type of storage is system-provided storage, like a `/tmp` folder.
This is the type of storage that you should ideally use for all your temporary tasks.
The fact that each node has its own temporary storage makes scaling significantly easier.
This storage is also very often SSD-based, thus is significantly faster.
The local storage can easily be configured for the application with
the usage of `TMPDIR` variable.
- **Shared temporary storage** (short-term storage) This type of storage is network-based temporary storage,
usually run with a common NFS server. As of Feb 2020, we still use this type of storage
for most of our implementations. Even though this allows the above limit to be significantly larger,
it does not really mean that you can use more. The shared temporary storage is shared by
all nodes. Thus, the job that uses significant amount of that space or performs a lot
of operations creates a contention on execution of all other jobs and request
across the whole application, this can easily impact stability of the whole GitLab.
Be respectful of that.
- **Shared persistent storage** (long-term storage) This type of storage uses
shared network-based storage (for example, NFS). This solution is mostly used by customers running small
installations consisting of a few nodes. The files on shared storage are easily accessible,
but any job that is uploading or downloading data can create a serious contention to all other jobs.
This is also an approach by default used by Omnibus.
- **Object-based persistent storage** (long term storage) this type of storage uses external
services like [AWS S3](https://en.wikipedia.org/wiki/Amazon_S3). The Object Storage
can be treated as infinitely scalable and redundant. Accessing this storage usually requires
downloading the file to manipulate it. The Object Storage can be considered as an ultimate
solution, as by definition it can be assumed that it can handle unlimited concurrent uploads
and downloads of files. This is also ultimate solution required to ensure that application can
run in containerized deployments (Kubernetes) at ease.
### Temporary storage
The storage on production nodes is really sparse. The application should be built
in a way that accommodates running under very limited temporary storage.
You can expect the system on which your code runs has a total of `1G-10G`
of temporary storage. However, this storage is really shared across all
jobs being run. If your job requires to use more than `100MB` of that space
you should reconsider the approach you have taken.
Whatever your needs are, you should clearly document if you need to process files.
If you require more than `100MB`, consider asking for help from a maintainer
to work with you to possibly discover a better solution.
#### Local temporary storage
The usage of local storage is a desired solution to use,
especially since we work on deploying applications to Kubernetes clusters.
When you would like to use `Dir.mktmpdir`? In a case when you want for example
to extract/create archives, perform extensive manipulation of existing data, and so on.
```ruby
Dir.mktmpdir('designs') do |path|
# do manipulation on path
# the path will be removed once
# we go out of the block
end
```
#### Shared temporary storage
The usage of shared temporary storage is required if your intent
is to persistent file for a disk-based storage, and not Object Storage.
[Workhorse direct upload](uploads/index.md#direct-upload) when accepting file
can write it to shared storage, and later GitLab Rails can perform a move operation.
The move operation on the same destination is instantaneous.
The system instead of performing `copy` operation just re-attaches file into a new place.
Since this introduces extra complexity into application, you should only try
to re-use well established patterns (for example, `ObjectStorage` concern) instead of re-implementing it.
The usage of shared temporary storage is otherwise deprecated for all other usages.
### Persistent storage
#### Object Storage
It is required that all features holding persistent files support saving data
to Object Storage. Having a persistent storage in the form of shared volume across nodes
is not scalable, as it creates a contention on data access all nodes.
GitLab offers the [ObjectStorage concern](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/uploaders/object_storage.rb)
that implements a seamless support for Shared and Object Storage-based persistent storage.
#### Data access
Each feature that accepts data uploads or allows to download them needs to use
[Workhorse direct upload](uploads/index.md#direct-upload). It means that uploads needs to be
saved directly to Object Storage by Workhorse, and all downloads needs to be served
by Workhorse.
Performing uploads/downloads via Puma is an expensive operation,
as it blocks the whole processing slot (thread) for the duration of the upload.
Performing uploads/downloads via Puma also has a problem where the operation
can time out, which is especially problematic for slow clients. If clients take a long time
to upload/download the processing slot might be killed due to request processing
timeout (usually between 30s-60s).
For the above reasons it is required that [Workhorse direct upload](uploads/index.md#direct-upload) is implemented
for all file uploads and downloads.
<!-- This redirect file can be deleted after <2023-04-10>. -->
<!-- Redirects that point to other docs in the same project expire in three months. -->
<!-- Redirects that point to docs in a different project or site (for example, link is not relative and starts with `https:`) expire in one year. -->
<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/redirects.html -->

2
doc/development/pages/index.md
View File

@ -147,7 +147,7 @@ GitLab Pages access control is disabled by default. To enable it:
1. Restart GitLab (if running through the GDK, run `gdk restart`). Running
`gdk reconfigure` overwrites the value of `access_control` in `config/gitlab.yml`.
1. In your local GitLab instance, in the browser go to `http://gdk.test:3000/admin/applications`.
1. Create an [Instance-wide OAuth application](../../integration/oauth_provider.md#instance-wide-applications)
1. Create an [Instance-wide OAuth application](../../integration/oauth_provider.md#create-an-instance-wide-application)
with the `api` scope.
1. Set the value of your `redirect-uri` to the `pages-domain` authorization endpoint
(for example, `http://pages.gdk.test:3010/auth`).

4
doc/development/performance.md
View File

@ -14,7 +14,7 @@ consistent performance of GitLab. Refer to the [Index](#performance-documentatio
- General:
- [Solving performance issues](#workflow)
- [Handbook performance page](https://about.gitlab.com/handbook/engineering/performance/)
- [Merge request performance guidelines](../development/merge_request_performance_guidelines.md)
- [Merge request performance guidelines](merge_request_concepts/performance.md)
- Backend:
- [Tooling](#tooling)
- Database:
@ -51,7 +51,7 @@ The process of solving performance problems is roughly as follows:
1. Add your findings based on the measurement period (screenshots of graphs,
timings, etc) to the issue mentioned in step 1.
1. Solve the problem.
1. Create a merge request, assign the "Performance" label and follow the [performance review process](merge_request_performance_guidelines.md).
1. Create a merge request, assign the "Performance" label and follow the [performance review process](merge_request_concepts/performance.md).
1. Once a change has been deployed make sure to _again_ measure for at least 24
hours to see if your changes have any impact on the production environment.
1. Repeat until you're done.

2
doc/development/value_stream_analytics/value_stream_analytics_aggregated_backend.md
View File

@ -273,7 +273,7 @@ attributes.
- `stages` - Load the stages for the currently selected value stream.
- `median` - For each stage, request the median duration.
- `count` - For each stage, request the number of items in the stage (this is a
[limit count](../merge_request_performance_guidelines.md#badge-counters), maximum 1000 rows).
[limit count](../merge_request_concepts/performance.md#badge-counters), maximum 1000 rows).
- `average_duration_chart` - Data for the duration chart.
- `summary`, `time_summary` - Top-level aggregations, most of the metrics are using different APIs/
finders and not invoking the aggregated backend.

8
doc/integration/jira/dvcs.md
View File

@ -72,14 +72,14 @@ you can still perform multiple actions in a single commit. For example:
## Configure a GitLab application for DVCS
For projects in a single group we recommend you create a [group application](../oauth_provider.md#group-owned-applications).
For projects in a single group we recommend you create a [group application](../oauth_provider.md#create-a-group-owned-application).
For projects across multiple groups we recommend you create and use a `jira` user in GitLab, and use the account
only for integration work. A separate account ensures regular account
maintenance does not affect your integration. If a `jira` user or group application is not feasible,
you can set up this integration as an [instance-wide application](../oauth_provider.md#instance-wide-applications)
or with a [user owned application](../oauth_provider.md#user-owned-applications) instead.
you can set up this integration as an [instance-wide application](../oauth_provider.md#create-an-instance-wide-application)
or with a [user owned application](../oauth_provider.md#create-a-user-owned-application) instead.
1. Navigate to the [appropriate **Applications** section](../oauth_provider.md#introduction-to-oauth).
1. Navigate to the [appropriate **Applications** section](../oauth_provider.md).
1. In the **Name** field, enter a descriptive name for the integration, such as `Jira`.
1. In the **Redirect URI** field, enter the URI appropriate for your version of GitLab,
replacing `<gitlab.example.com>` with your GitLab instance domain:

68
doc/integration/oauth_provider.md
View File

@ -6,17 +6,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w
# Configure GitLab as an OAuth 2.0 authentication identity provider
This document describes how you can use GitLab as an OAuth 2.0 authentication identity provider.
- OAuth 2 applications can be created and managed using the GitLab UI (described below)
or managed using the [Applications API](../api/applications.md).
- After an application is created, external services can manage access tokens using the
[OAuth 2 API](../api/oauth2.md).
- To allow users to sign in to GitLab using third-party OAuth 2 providers, see
[OmniAuth documentation](omniauth.md).
## Introduction to OAuth
[OAuth 2](https://oauth.net/2/) provides to client applications a 'secure delegated
access' to server resources on behalf of a resource owner. OAuth 2 allows
authorization servers to issue access tokens to third-party clients with the approval
@ -33,21 +22,30 @@ to repositories without sharing user credentials to your GitLab.com account.
GitLab supports several ways of adding a new OAuth 2 application to an instance:
- [User owned applications](#user-owned-applications)
- [Group owned applications](#group-owned-applications)
- [Instance-wide applications](#instance-wide-applications)
- [User owned applications](#create-a-user-owned-application)
- [Group owned applications](#create-a-group-owned-application)
- [Instance-wide applications](#create-an-instance-wide-application)
The only difference between these methods is the [permission](../user/permissions.md)
levels. The default callback URL is `https://your-gitlab.example.com/users/auth/gitlab/callback` (you can also use a non-SSL URL, but you should use SSL URLs).
## User owned applications
This document describes how you can use GitLab as an OAuth 2.0 authentication identity provider.
- OAuth 2 applications can be created and managed using the GitLab UI (described below)
or managed using the [Applications API](../api/applications.md).
- After an application is created, external services can manage access tokens using the
[OAuth 2 API](../api/oauth2.md).
- To allow users to sign in to GitLab using third-party OAuth 2 providers, see
[OmniAuth documentation](omniauth.md).
## Create a user-owned application
To add a new application for your user:
1. In the top-right corner, select your avatar.
1. Select **Edit profile**.
1. On the left sidebar, select **Applications**.
1. Enter a **Name**, **Redirect URI** and OAuth 2 scopes as defined in [Authorized Applications](#authorized-applications).
1. Enter a **Name**, **Redirect URI** and OAuth 2 scopes as defined in [Authorized Applications](#view-all-authorized-applications).
The **Redirect URI** is the URL where users are sent after they authorize with GitLab.
1. Select **Save application**. GitLab provides:
@ -57,7 +55,7 @@ To add a new application for your user:
- By selecting **Copy** in the **Secret** field
[in GitLab 14.2 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/332844).
## Group owned applications
## Create a group-owned application
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/16227) in GitLab 13.11.
@ -65,7 +63,7 @@ To add a new application for a group:
1. Go to the desired group.
1. On the left sidebar, select **Settings > Applications**.
1. Enter a **Name**, **Redirect URI** and OAuth 2 scopes as defined in [Authorized Applications](#authorized-applications).
1. Enter a **Name**, **Redirect URI** and OAuth 2 scopes as defined in [Authorized Applications](#view-all-authorized-applications).
The **Redirect URI** is the URL where users are sent after they authorize with GitLab.
1. Select **Save application**. GitLab provides:
@ -75,7 +73,7 @@ To add a new application for a group:
- By selecting **Copy** in the **Secret** field
[in GitLab 14.2 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/332844).
## Instance-wide applications
## Create an instance-wide application
To create an application for your GitLab instance:
@ -86,22 +84,7 @@ To create an application for your GitLab instance:
When creating application in the **Admin Area** , you can mark it as _trusted_.
The user authorization step is automatically skipped for this application.
## Access token expiration
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/21745) in GitLab 14.3, with the ability to opt out.
> - Ability to opt-out of expiring access token [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/340848) in GitLab 15.0.
WARNING:
The ability to opt-out of expiring access tokens was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/340848)
in GitLab 14.3 and [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/340848) in 15.0. All
existing integrations must be updated to support access token refresh.
Access tokens expire after two hours. Integrations that use access tokens must generate new ones at least every
two hours.
When applications are deleted, all grants and tokens associated with the application are also deleted.
## Authorized applications
## View all authorized applications
To see all the application you've authorized with your GitLab credentials:
@ -128,6 +111,21 @@ application can perform. Available scopes are depicted in the following table.
At any time you can revoke any access by selecting **Revoke**.
## Access token expiration
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/21745) in GitLab 14.3, with the ability to opt out.
> - Ability to opt-out of expiring access token [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/340848) in GitLab 15.0.
WARNING:
The ability to opt out of expiring access tokens was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/340848)
in GitLab 14.3 and [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/340848) in 15.0. All
existing integrations must be updated to support access token refresh.
Access tokens expire after two hours. Integrations that use access tokens must generate new ones at least every
two hours.
When applications are deleted, all grants and tokens associated with the application are also deleted.
## Hashed OAuth application secrets
> Introduced in GitLab 15.4 [with a flag](../administration/feature_flags.md) named `hash_oauth_secrets`. Disabled by default.

55
doc/policy/alpha-beta-support.md
View File

@ -4,39 +4,60 @@ group: Distribution
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
---
<!-- any changes made to this page should be reflected in https://about.gitlab.com/support/statement-of-support/#alpha-beta-features and https://about.gitlab.com/handbook/product/gitlab-the-product/#alpha-beta-ga -->
# Support for Alpha, Beta, Limited Availability, and Generally Available features **(PREMIUM)**
# Support for Alpha, Beta, Limited Availability, and Generally Available Features **(PREMIUM)**
Some GitLab features are released as Alpha or Beta versions and are
[not fully supported](https://about.gitlab.com/support/statement-of-support/#alpha-beta-features).
All other features are considered to be Generally Available (GA).
Some GitLab features are released as [Alpha or Beta versions](https://about.gitlab.com/support/statement-of-support/#alpha-beta-features) which are not fully supported. All other features are considered to be Generally Available (GA).
## Alpha features
## Alpha Features
Support is **not** provided for Alpha features and issues with them should be opened in the [GitLab issue tracker](https://gitlab.com/gitlab-org/gitlab/issues).
Characteristics of alpha features:
Characteristics of Alpha features:
- Not ready for production use.
- Unstable and can cause performance and stability issues.
- Configuration and dependencies are likely to change.
- Features and functions may be removed.
- Data loss can occur (be that through bugs or updates).
- Documentation reflects the Alpha status.
- Behind flags that are off by default.
- Not announced in release posts.
Support is **not** provided for Alpha features and issues with them should be opened in the [GitLab issue tracker](https://gitlab.com/gitlab-org/gitlab/issues).
## Beta Features
Characteristics of beta features:
- Not ready for production use.
- Unstable and can cause performance and stability issues.
- Configuration and dependencies are not likely to change.
- Features and functions are not likely to change.
- Data loss is not likely.
## Beta features
Your Support Contract provides **commercially-reasonable effort** support for Beta features, with the expectation that issues require extra time and assistance from development to troubleshoot.
### Closed Beta features
Closed Beta features are available to selected users only.
- Not ready for production use.
- Unstable and can cause performance and stability issues.
- Configuration and dependencies unlikely to change.
- Features and functions unlikely to change.
- Data loss less likely.
- Behind a feature flag that is off by default and the UI reflects Beta status.
- Documentation reflects Beta status.
- Can be announced in a release post that reflects Beta status.
### Open Beta features
- Not ready for production use.
- Unstable and can cause performance and stability issues.
- Configuration and dependencies unlikely to change.
- Features and functions unlikely to change.
- Data loss not likely.
- Support on a commercially-reasonable effort basis.
- Documentation reflects Beta status.
- Behind a feature flag that is on by default and the UI reflects Beta status.
- Behind a toggle that is off by default and the UI reflects Beta status.
- Can be announced in a release post that reflects Beta status.
## Limited Availability (LA)
Characteristics of limited availability features:
Characteristics of Limited Availability features:
- Ready for production use by a small set of customers.
- Can be booked by Deal Desk as part of an order.

8
doc/security/rate_limits.md
View File

@ -74,13 +74,7 @@ For configuration information, see
### Git operations using SSH
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78373) in GitLab 14.7 [with a flag](../administration/feature_flags.md) named `rate_limit_gitlab_shell`. Disabled by default.
> - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79419) in GitLab 14.8.
FLAG:
On self-managed GitLab, by default this feature is available. To disable the feature, ask an administrator to
[disable the feature flag](../administration/feature_flags.md) named `rate_limit_gitlab_shell`. On GitLab.com, this feature
is available.
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78373) in GitLab 14.7 [with a flag](../administration/feature_flags.md) named `rate_limit_gitlab_shell`. Available by default without a feature flag from 15.8.
GitLab applies rate limits to Git operations that use SSH by user account and project. When the rate limit is exceeded, GitLab rejects
further connection requests from that user for the project.

2
doc/topics/authentication/index.md
View File

@ -17,7 +17,7 @@ This page gathers all the resources for the topic **Authentication** within GitL
- [Support for Universal 2nd Factor Authentication - YubiKeys](https://about.gitlab.com/blog/2016/06/22/gitlab-adds-support-for-u2f/)
- [Security Webcast with Yubico](https://about.gitlab.com/blog/2016/08/31/gitlab-and-yubico-security-webcast/)
- **Integrations:**
- [GitLab as OAuth2 authentication service provider](../../integration/oauth_provider.md#introduction-to-oauth)
- [GitLab as OAuth2 authentication service provider](../../integration/oauth_provider.md)
- [GitLab as OpenID Connect identity provider](../../integration/openid_connect_provider.md)
## GitLab administrators

2
doc/user/application_security/secret_detection/post_processing.md
View File

@ -11,7 +11,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
FLAG:
By default, auto revocation of GitLab personal access tokens is not available. To opt-in on GitLab.com
during the [Beta period](https://about.gitlab.com/handbook/product/gitlab-the-product/#alpha-beta-ga), please
during the [Beta period](../../../policy/alpha-beta-support.md#beta-features), please
[let us know by completing this form](https://docs.google.com/forms/d/e/1FAIpQLSdRbFhvA5jvI-Rt_Qnl1PQ1znOXKK8m6lRtmM0uva4upetKvQ/viewform).
GitLab supports running post-processing hooks after detecting a secret. These

2
doc/user/profile/index.md
View File

@ -391,7 +391,7 @@ Without the `config.extend_remember_period` flag, you would be forced to sign in
- Receive emails for:
- [Sign-ins from unknown IP addresses or devices](notifications.md#notifications-for-unknown-sign-ins)
- [Attempted sign-ins using wrong two-factor authentication codes](notifications.md#notifications-for-attempted-sign-in-using-wrong-two-factor-authentication-codes)
- Manage applications that can [use GitLab as an OAuth provider](../../integration/oauth_provider.md#introduction-to-oauth)
- Manage applications that can [use GitLab as an OAuth provider](../../integration/oauth_provider.md)
- Manage [personal access tokens](personal_access_tokens.md) to access your account via API and authorized applications
- Manage [SSH keys](../ssh.md) to access your account via SSH
- Change your [syntax highlighting theme](preferences.md#syntax-highlighting-theme)

2
doc/user/project/merge_requests/versions.md
View File

@ -64,7 +64,7 @@ branches. The new mode is available from the comparison target drop down
by selecting **main (HEAD)**. In GitLab 13.9, it
[replaced](https://gitlab.com/gitlab-org/gitlab/-/issues/198458) the
old default comparison. For technical details, additional information is available in the
[developer documentation](../../../development/diffs.md#merge-request-diffs-against-the-head-of-the-target-branch).
[developer documentation](../../../development/merge_request_concepts/diffs/index.md#merge-request-diffs-against-the-head-of-the-target-branch).
![Merge request versions compare HEAD](img/versions_compare_head_v12_10.png)

4
lib/api/internal/base.rb
View File

@ -44,9 +44,7 @@ module API
# This is a separate method so that EE can alter its behaviour more
# easily.
if Feature.enabled?(:rate_limit_gitlab_shell)
check_rate_limit!(:gitlab_shell_operation, scope: [params[:action], params[:project], actor.key_or_user])
end
check_rate_limit!(:gitlab_shell_operation, scope: [params[:action], params[:project], actor.key_or_user])
rate_limiter = Gitlab::Auth::IpRateLimiter.new(request.ip)

2
lib/gitlab/auth.rb
View File

@ -31,6 +31,7 @@ module Gitlab
# Scopes used for GitLab as admin
SUDO_SCOPE = :sudo
ADMIN_MODE_SCOPE = :admin_mode
ADMIN_SCOPES = [SUDO_SCOPE].freeze
# Default scopes for OAuth applications that don't define their own
@ -366,6 +367,7 @@ module Gitlab
def available_scopes_for(current_user)
scopes = non_admin_available_scopes
scopes += ADMIN_SCOPES if current_user.admin?
scopes
end

28
lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens.rb Normal file
View File

@ -0,0 +1,28 @@
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# Backfill `admin_mode` scope for a range of personal access tokens
class BackfillAdminModeScopeForPersonalAccessTokens < ::Gitlab::BackgroundMigration::BatchedMigrationJob
scope_to ->(relation) do
relation.joins('INNER JOIN users ON personal_access_tokens.user_id = users.id')
.where(users: { admin: true })
.where(revoked: [false, nil])
.where.not('expires_at IS NOT NULL AND expires_at <= ?', Time.current)
end
operation_name :update_all
feature_category :authentication_and_authorization
ADMIN_MODE_SCOPE = ['admin_mode'].freeze
def perform
each_sub_batch do |sub_batch|
sub_batch.each do |token|
token.update!(scopes: (YAML.safe_load(token.scopes) + ADMIN_MODE_SCOPE).uniq.to_yaml)
end
end
end
end
end
end

24
lib/gitlab/database/migration_helpers.rb
View File

@ -357,10 +357,28 @@ module Gitlab
end
def foreign_key_exists?(source, target = nil, **options)
foreign_keys(source).any? do |foreign_key|
tables_match?(target.to_s, foreign_key.to_table.to_s) &&
options_match?(foreign_key.options, options)
# This if block is necessary because foreign_key_exists? is called in down migrations that may execute before
# the postgres_foreign_keys view had necessary columns added, or even before the view existed.
# In that case, we revert to the previous behavior of this method.
# The behavior in the if block has a bug: it always returns false if the fk being checked has multiple columns.
# This can be removed after init_schema.rb passes 20221122210711_add_columns_to_postgres_foreign_keys.rb
# Tracking issue: https://gitlab.com/gitlab-org/gitlab/-/issues/386796
if ActiveRecord::Migrator.current_version < 20221122210711
return foreign_keys(source).any? do |foreign_key|
tables_match?(target.to_s, foreign_key.to_table.to_s) &&
options_match?(foreign_key.options, options)
end
end
fks = Gitlab::Database::PostgresForeignKey.by_constrained_table_name(source)
fks = fks.by_referenced_table_name(target) if target
fks = fks.by_name(options[:name]) if options[:name]
fks = fks.by_constrained_columns(options[:column]) if options[:column]
fks = fks.by_referenced_columns(options[:primary_key]) if options[:primary_key]
fks = fks.by_on_delete_action(options[:on_delete]) if options[:on_delete]
fks.exists?
end
# Returns the name for a concurrent foreign key.

18
lib/gitlab/database/postgres_foreign_key.rb
View File

@ -9,7 +9,7 @@ module Gitlab
enum on_delete_action: {
restrict: 'r',
cascade: 'c',
set_null: 'n',
nullify: 'n',
set_default: 'd',
no_action: 'a'
}
@ -20,13 +20,29 @@ module Gitlab
where(referenced_table_identifier: identifier)
end
scope :by_referenced_table_name, ->(name) { where(referenced_table_name: name) }
scope :by_constrained_table_identifier, ->(identifier) do
raise ArgumentError, "Constrained table name is not fully qualified with a schema: #{identifier}" unless identifier =~ /^\w+\.\w+$/
where(constrained_table_identifier: identifier)
end
scope :by_constrained_table_name, ->(name) { where(constrained_table_name: name) }
scope :not_inherited, -> { where(is_inherited: false) }
scope :by_name, ->(name) { where(name: name) }
scope :by_constrained_columns, ->(cols) { where(constrained_columns: Array.wrap(cols)) }
scope :by_referenced_columns, ->(cols) { where(referenced_columns: Array.wrap(cols)) }
scope :by_on_delete_action, ->(on_delete) do
raise ArgumentError, "Invalid on_delete action #{on_delete}" unless on_delete_actions.key?(on_delete)
where(on_delete_action: on_delete)
end
end
end
end

2
lib/gitlab/hotlinking_detector.rb
View File

@ -12,8 +12,6 @@ module Gitlab
def intercept_hotlinking?(request)
request_accepts = parse_request_accepts(request)
return false unless Feature.enabled?(:repository_archive_hotlinking_interception)
# Block attempts to embed as JS
return true if sec_fetch_invalid?(request)

20
locale/gitlab.pot
View File

@ -16662,7 +16662,7 @@ msgstr ""
msgid "Expires:"
msgstr ""
msgid "Explain the problem. If appropriate, provide a link to the relevant issue or comment."
msgid "Explain why you're reporting the user."
msgstr ""
msgid "Explore"
@ -24423,9 +24423,6 @@ msgstr ""
msgid "Last successful update %{time}."
msgstr ""
msgid "Last time verified"
msgstr ""
msgid "Last update"
msgstr ""
@ -31423,6 +31420,9 @@ msgstr ""
msgid "Please type %{phrase_code} to proceed or close this modal to cancel."
msgstr ""
msgid "Please use this form to report to the administrator users who create spam issues, comments or behave inappropriately."
msgstr ""
msgid "Please wait a few moments while we load the file history for this line."
msgstr ""
@ -35847,9 +35847,6 @@ msgstr ""
msgid "Resume"
msgstr ""
msgid "Resync"
msgstr ""
msgid "Retrieving the compliance report failed. Refresh the page and try again."
msgstr ""
@ -45380,9 +45377,6 @@ msgstr ""
msgid "Use the search bar on the top of this page"
msgstr ""
msgid "Use this form to report to the administrator users who create spam issues, comments or behave inappropriately."
msgstr ""
msgid "Use this token to validate received payloads."
msgstr ""
@ -48215,9 +48209,6 @@ msgid_plural "You have %{pendingMembersCount} pending members that need approval
msgstr[0] ""
msgstr[1] ""
msgid "You have already reported this user"
msgstr ""
msgid "You have been granted %{access_level} access to the %{source_link} %{source_type}."
msgstr ""
@ -49821,6 +49812,9 @@ msgstr[1] ""
msgid "has already been linked to another vulnerability"
msgstr ""
msgid "has already been reported for abuse"
msgstr ""
msgid "has already been taken"
msgstr ""

1
qa/qa/specs/features/browser_ui/1_manage/user/parent_group_access_termination_spec.rb
View File

@ -34,6 +34,7 @@ module QA
Page::Group::Menu.perform(&:click_subgroup_members_item)
Page::Group::Members.perform do |members_page|
members_page.search_member(user.username)
members_page.remove_member(user.username)
end
end

1
spec/factories/abuse_reports.rb
View File

@ -5,5 +5,6 @@ FactoryBot.define do
reporter factory: :user
user
message { 'User sends spam' }
reported_from_url { 'http://gitlab.com' }
end
end

201
spec/features/abuse_report_spec.rb
View File

@ -2,130 +2,119 @@
require 'spec_helper'
RSpec.describe 'Abuse reports', feature_category: :insider_threat do
let_it_be(:abusive_user) { create(:user, username: 'abuser_mcabusive') }
let_it_be(:reporter1) { create(:user, username: 'reporter_mcreporty') }
let_it_be(:reporter2) { create(:user) }
RSpec.describe 'Abuse reports', :js, feature_category: :insider_threat do
let_it_be(:abusive_user) { create(:user) }
let_it_be(:reporter1) { create(:user) }
let_it_be(:project) { create(:project, :public) }
let_it_be(:issue) { create(:issue, project: project, author: abusive_user) }
let!(:group) do
create(:group).tap do |g|
g.add_owner(reporter1)
g.add_developer(abusive_user)
end
end
before do
sign_in(reporter1)
end
it 'allows a user to be reported for abuse from an issue', :js do
visit project_issue_path(project, issue)
context 'when reporting an issue for abuse' do
it 'allows a user to be reported for abuse from an issue', :js do
visit project_issue_path(project, issue)
click_button 'Issue actions'
click_link 'Report abuse to administrator'
click_button 'Issue actions'
click_link 'Report abuse to administrator'
wait_for_requests
wait_for_requests
fill_and_submit_form
fill_and_submit_form
expect(page).to have_content 'Thank you for your report'
end
it 'allows a user to be reported for abuse from their profile', :js do
visit user_path(abusive_user)
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
wait_for_requests
fill_and_submit_form
expect(page).to have_content 'Thank you for your report'
visit user_path(abusive_user)
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
fill_and_submit_form
expect(page).to have_content 'You have already reported this user'
end
it 'allows multiple users to report a user', :js do
visit user_path(abusive_user)
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
wait_for_requests
fill_and_submit_form
expect(page).to have_content 'Thank you for your report'
sign_out(reporter1)
sign_in(reporter2)
visit user_path(abusive_user)
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
wait_for_requests
fill_and_submit_form
expect(page).to have_content 'Thank you for your report'
end
describe 'Cancel', :js do
context 'when ref_url is not present (e.g. visit user page then click on report abuse)' do
it 'links the user back to where abuse report was triggered' do
origin_url = user_path(abusive_user)
visit origin_url
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
wait_for_requests
click_link 'Cancel'
expect(page).to have_current_path(origin_url)
end
expect(page).to have_content 'Thank you for your report'
end
context 'when ref_url is present (e.g. user is reported from one of their MRs)' do
it 'links the user back to ref_url' do
ref_url = group_group_members_path(group)
it 'redirects backs to the issue when cancel button is clicked' do
visit project_issue_path(project, issue)
visit ref_url
click_button 'Issue actions'
click_link 'Report abuse to administrator'
# visit abusive user's profile page
page.first('.js-user-link').click
wait_for_requests
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
click_link 'Cancel'
click_link 'Cancel'
expect(page).to have_current_path(project_issue_path(project, issue))
end
end
expect(page).to have_current_path(ref_url)
end
context 'when reporting a user profile for abuse' do
let_it_be(:reporter2) { create(:user) }
it 'allows a user to be reported for abuse from their profile' do
visit user_path(abusive_user)
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
wait_for_requests
fill_and_submit_form
expect(page).to have_content 'Thank you for your report'
visit user_path(abusive_user)
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
fill_and_submit_form
expect(page).to have_content 'User has already been reported for abuse'
end
it 'allows multiple users to report a user' do
visit user_path(abusive_user)
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
wait_for_requests
fill_and_submit_form
expect(page).to have_content 'Thank you for your report'
gitlab_sign_out
gitlab_sign_in(reporter2)
visit user_path(abusive_user)
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
wait_for_requests
fill_and_submit_form
expect(page).to have_content 'Thank you for your report'
end
it 'redirects backs to user profile when cancel button is clicked' do
visit user_path(abusive_user)
click_button 'Report abuse to administrator'
choose "They're posting spam."
click_button 'Next'
wait_for_requests
click_link 'Cancel'
expect(page).to have_current_path(user_path(abusive_user))
end
end

23
spec/features/merge_request/user_sees_discussions_navigation_spec.rb
View File

@ -194,29 +194,10 @@ RSpec.describe 'Merge request > User sees discussions navigation', :js, feature_
end
def goto_next_thread
begin
# this is required when moved_mr_sidebar is enabled
page.within('.issue-sticky-header') do
click_button 'Go to next unresolved thread'
end
rescue StandardError
click_button 'Go to next unresolved thread'
end
wait_for_scroll_end
click_button 'Go to next unresolved thread'
end
def goto_previous_thread
begin
page.within('.issue-sticky-header') do
click_button 'Go to previous unresolved thread'
end
rescue StandardError
click_button 'Go to previous unresolved thread'
end
wait_for_scroll_end
end
def wait_for_scroll_end
sleep(1)
click_button 'Go to previous unresolved thread'
end
end

2
spec/frontend/abuse_reports/components/abuse_category_selector_spec.js
View File

@ -112,7 +112,7 @@ describe('AbuseCategorySelector', () => {
it('renders referer as a hidden fields', () => {
expect(findReferer().attributes()).toMatchObject({
type: 'hidden',
name: 'ref_url',
name: 'abuse_report[reported_from_url]',
value: REPORTED_FROM_URL,
});
});

30
spec/frontend/constants_spec.js Normal file
View File

@ -0,0 +1,30 @@
import * as constants from '~/constants';
describe('Global JS constants', () => {
describe('getModifierKey()', () => {
afterEach(() => {
delete window.gl;
});
it.each`
isMac | removeSuffix | expectedKey
${true} | ${false} | ${'⌘'}
${false} | ${false} | ${'Ctrl+'}
${true} | ${true} | ${'⌘'}
${false} | ${true} | ${'Ctrl'}
`(
'returns correct keystroke when isMac=$isMac and removeSuffix=$removeSuffix',
({ isMac, removeSuffix, expectedKey }) => {
Object.assign(window, {
gl: {
client: {
isMac,
},
},
});
expect(constants.getModifierKey(removeSuffix)).toBe(expectedKey);
},
);
});
});

8
spec/frontend/packages_and_registries/container_registry/explorer/components/details_page/tags_list_row_spec.js
View File

@ -335,10 +335,10 @@ describe('tags list row', () => {
});
describe.each`
name | finderFunction | text | icon | clipboard
${'published date detail'} | ${findPublishedDateDetail} | ${'Published to the gitlab-org/gitlab-test/rails-12009 image repository at 01:29 UTC on 2020-11-03'} | ${'clock'} | ${false}
${'manifest detail'} | ${findManifestDetail} | ${'Manifest digest: sha256:2cf3d2fdac1b04a14301d47d51cb88dcd26714c74f91440eeee99ce399089062'} | ${'log'} | ${true}
${'configuration detail'} | ${findConfigurationDetail} | ${'Configuration digest: sha256:c2613843ab33aabf847965442b13a8b55a56ae28837ce182627c0716eb08c02b'} | ${'cloud-gear'} | ${true}
name | finderFunction | text | icon | clipboard
${'published date detail'} | ${findPublishedDateDetail} | ${'Published to the gitlab-org/gitlab-test/rails-12009 image repository at 13:29:38 UTC on 2020-11-03'} | ${'clock'} | ${false}
${'manifest detail'} | ${findManifestDetail} | ${'Manifest digest: sha256:2cf3d2fdac1b04a14301d47d51cb88dcd26714c74f91440eeee99ce399089062'} | ${'log'} | ${true}
${'configuration detail'} | ${findConfigurationDetail} | ${'Configuration digest: sha256:c2613843ab33aabf847965442b13a8b55a56ae28837ce182627c0716eb08c02b'} | ${'cloud-gear'} | ${true}
`('$name details row', ({ finderFunction, text, icon, clipboard }) => {
it(`has ${text} as text`, async () => {
mountComponent();

16
spec/frontend/packages_and_registries/infrastructure_registry/components/details/components/package_history_spec.js
View File

@ -61,14 +61,14 @@ describe('Package History', () => {
);
});
describe.each`
name | amount | icon | text | timeAgoTooltip | link
${'created-on'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'clock'} | ${'Test package version 1.0.0 was first created'} | ${mavenPackage.created_at} | ${null}
${'first-pipeline-commit'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'commit'} | ${'Created by commit #sha-baz on branch branch-name'} | ${null} | ${mockPipelineInfo.project.commit_url}
${'first-pipeline-pipeline'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'pipeline'} | ${'Built by pipeline #1 triggered by foo'} | ${mockPipelineInfo.created_at} | ${mockPipelineInfo.project.pipeline_url}
${'published'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'package'} | ${'Published to the baz project Package Registry'} | ${mavenPackage.created_at} | ${null}
${'archived'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'history'} | ${'Package has 1 archived update'} | ${null} | ${null}
${'archived'} | ${HISTORY_PIPELINES_LIMIT + 3} | ${'history'} | ${'Package has 2 archived updates'} | ${null} | ${null}
${'pipeline-entry'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'pencil'} | ${'Package updated by commit #sha-baz on branch branch-name, built by pipeline #3, and published to the registry'} | ${mavenPackage.created_at} | ${mockPipelineInfo.project.commit_url}
name | amount | icon | text | timeAgoTooltip | link
${'created-on'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'clock'} | ${'Test package version 1.0.0 was first created'} | ${mavenPackage.created_at} | ${null}
${'first-pipeline-commit'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'commit'} | ${'Created by commit sha-baz on branch branch-name'} | ${null} | ${mockPipelineInfo.project.commit_url}
${'first-pipeline-pipeline'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'pipeline'} | ${'Built by pipeline #1 triggered by foo'} | ${mockPipelineInfo.created_at} | ${mockPipelineInfo.project.pipeline_url}
${'published'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'package'} | ${'Published to the baz project Package Registry'} | ${mavenPackage.created_at} | ${null}
${'archived'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'history'} | ${'Package has 1 archived update'} | ${null} | ${null}
${'archived'} | ${HISTORY_PIPELINES_LIMIT + 3} | ${'history'} | ${'Package has 2 archived updates'} | ${null} | ${null}
${'pipeline-entry'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'pencil'} | ${'Package updated by commit sha-baz on branch branch-name, built by pipeline #3, and published to the registry'} | ${mavenPackage.created_at} | ${mockPipelineInfo.project.commit_url}
`(
'with $amount pipelines history element $name',
({ name, icon, text, timeAgoTooltip, link, amount }) => {

16
spec/frontend/packages_and_registries/package_registry/components/details/package_history_spec.js
View File

@ -131,14 +131,14 @@ describe('Package History', () => {
});
describe.each`
name | amount | icon | text | timeAgoTooltip | link
${'created-on'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'clock'} | ${'@gitlab-org/package-15 version 1.0.0 was first created'} | ${packageData().createdAt} | ${null}
${'first-pipeline-commit'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'commit'} | ${'Created by commit #b83d6e39 on branch master'} | ${null} | ${onePipeline.commitPath}
${'first-pipeline-pipeline'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'pipeline'} | ${'Built by pipeline #1 triggered by Administrator'} | ${onePipeline.createdAt} | ${onePipeline.path}
${'published'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'package'} | ${'Published to the baz project Package Registry'} | ${packageData().createdAt} | ${null}
${'archived'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'history'} | ${'Package has 1 archived update'} | ${null} | ${null}
${'archived'} | ${HISTORY_PIPELINES_LIMIT + 3} | ${'history'} | ${'Package has 2 archived updates'} | ${null} | ${null}
${'pipeline-entry'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'pencil'} | ${'Package updated by commit #b83d6e39 on branch master, built by pipeline #3, and published to the registry'} | ${packageData().createdAt} | ${onePipeline.commitPath}
name | amount | icon | text | timeAgoTooltip | link
${'created-on'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'clock'} | ${'@gitlab-org/package-15 version 1.0.0 was first created'} | ${packageData().createdAt} | ${null}
${'first-pipeline-commit'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'commit'} | ${'Created by commit b83d6e39 on branch master'} | ${null} | ${onePipeline.commitPath}
${'first-pipeline-pipeline'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'pipeline'} | ${'Built by pipeline #1 triggered by Administrator'} | ${onePipeline.createdAt} | ${onePipeline.path}
${'published'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'package'} | ${'Published to the baz project Package Registry'} | ${packageData().createdAt} | ${null}
${'archived'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'history'} | ${'Package has 1 archived update'} | ${null} | ${null}
${'archived'} | ${HISTORY_PIPELINES_LIMIT + 3} | ${'history'} | ${'Package has 2 archived updates'} | ${null} | ${null}
${'pipeline-entry'} | ${HISTORY_PIPELINES_LIMIT + 2} | ${'pencil'} | ${'Package updated by commit b83d6e39 on branch master, built by pipeline #3, and published to the registry'} | ${packageData().createdAt} | ${onePipeline.commitPath}
`(
'with $amount pipelines history element $name',
({ name, icon, text, timeAgoTooltip, link, amount }) => {

11
spec/frontend/users/profile/components/report_abuse_button_spec.js
View File

@ -1,5 +1,6 @@
import { GlButton } from '@gitlab/ui';
import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
import { BV_HIDE_TOOLTIP } from '~/lib/utils/constants';
import ReportAbuseButton from '~/users/profile/components/report_abuse_button.vue';
import AbuseCategorySelector from '~/abuse_reports/components/abuse_category_selector.vue';
@ -69,4 +70,14 @@ describe('ReportAbuseButton', () => {
expect(findAbuseCategorySelector().props('showDrawer')).toBe(false);
});
});
describe('when user hovers out of the button', () => {
it(`should emit ${BV_HIDE_TOOLTIP} to close the tooltip`, () => {
jest.spyOn(wrapper.vm.$root, '$emit');
findReportAbuseButton().vm.$emit('mouseout');
expect(wrapper.vm.$root.$emit).toHaveBeenCalledWith(BV_HIDE_TOOLTIP);
});
});
});

13
spec/frontend/work_items/components/work_item_notes_spec.js
View File

@ -6,6 +6,7 @@ import createMockApollo from 'helpers/mock_apollo_helper';
import waitForPromises from 'helpers/wait_for_promises';
import SystemNote from '~/work_items/components/notes/system_note.vue';
import WorkItemNotes from '~/work_items/components/work_item_notes.vue';
import WorkItemCommentForm from '~/work_items/components/work_item_comment_form.vue';
import ActivityFilter from '~/work_items/components/notes/activity_filter.vue';
import workItemNotesQuery from '~/work_items/graphql/work_item_notes.query.graphql';
import workItemNotesByIidQuery from '~/work_items/graphql/work_item_notes_by_iid.query.graphql';
@ -40,6 +41,7 @@ describe('WorkItemNotes component', () => {
const findAllSystemNotes = () => wrapper.findAllComponents(SystemNote);
const findActivityLabel = () => wrapper.find('label');
const findWorkItemCommentForm = () => wrapper.findComponent(WorkItemCommentForm);
const findSkeletonLoader = () => wrapper.findComponent(GlSkeletonLoader);
const findIntersectionObserver = () => wrapper.findComponent(GlIntersectionObserver);
const findSortingFilter = () => wrapper.findComponent(ActivityFilter);
@ -85,6 +87,13 @@ describe('WorkItemNotes component', () => {
expect(findActivityLabel().exists()).toBe(true);
});
it('passes correct props to comment form component', async () => {
createComponent({ workItemId: mockWorkItemId, fetchByIid: false });
await waitForPromises();
expect(findWorkItemCommentForm().props('fetchByIid')).toEqual(false);
});
describe('when notes are loading', () => {
it('renders skeleton loader', () => {
expect(findSkeletonLoader().exists()).toBe(true);
@ -117,6 +126,10 @@ describe('WorkItemNotes component', () => {
mockNotesByIidWidgetResponse.discussions.nodes.length,
);
});
it('passes correct props to comment form component', () => {
expect(findWorkItemCommentForm().props('fetchByIid')).toEqual(true);
});
});
describe('Pagination', () => {

53
spec/lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens_spec.rb Normal file
View File

@ -0,0 +1,53 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::BackgroundMigration::BackfillAdminModeScopeForPersonalAccessTokens,
:migration, schema: 20221228103133, feature_category: :authentication_and_authorization do
let(:users) { table(:users) }
let(:personal_access_tokens) { table(:personal_access_tokens) }
let(:admin) { users.create!(name: 'admin', email: 'admin@example.com', projects_limit: 1, admin: true) }
let(:user) { users.create!(name: 'user', email: 'user@example.com', projects_limit: 1) }
let!(:pat_admin_1) { personal_access_tokens.create!(name: 'admin 1', user_id: admin.id, scopes: "---\n- api\n") }
let!(:pat_user) { personal_access_tokens.create!(name: 'user 1', user_id: user.id, scopes: "---\n- api\n") }
let!(:pat_revoked) do
personal_access_tokens.create!(name: 'admin 2', user_id: admin.id, scopes: "---\n- api\n", revoked: true)
end
let!(:pat_expired) do
personal_access_tokens.create!(name: 'admin 3', user_id: admin.id, scopes: "---\n- api\n", expires_at: 1.day.ago)
end
let!(:pat_admin_mode) do
personal_access_tokens.create!(name: 'admin 4', user_id: admin.id, scopes: "---\n- admin_mode\n")
end
let!(:pat_admin_2) { personal_access_tokens.create!(name: 'admin 5', user_id: admin.id, scopes: "---\n- read_api\n") }
let!(:pat_not_in_range) { personal_access_tokens.create!(name: 'admin 6', user_id: admin.id, scopes: "---\n- api\n") }
subject do
described_class.new(
start_id: pat_admin_1.id,
end_id: pat_admin_2.id,
batch_table: :personal_access_tokens,
batch_column: :id,
sub_batch_size: 1,
pause_ms: 0,
connection: ApplicationRecord.connection
)
end
it "adds `admin_mode` scope to active personal access tokens of administrators" do
subject.perform
expect(pat_admin_1.reload.scopes).to eq("---\n- api\n- admin_mode\n")
expect(pat_user.reload.scopes).to eq("---\n- api\n")
expect(pat_revoked.reload.scopes).to eq("---\n- api\n")
expect(pat_expired.reload.scopes).to eq("---\n- api\n")
expect(pat_admin_mode.reload.scopes).to eq("---\n- admin_mode\n")
expect(pat_admin_2.reload.scopes).to eq("---\n- read_api\n- admin_mode\n")
expect(pat_not_in_range.reload.scopes).to eq("---\n- api\n")
end
end

93
spec/lib/gitlab/database/migration_helpers_spec.rb
View File

@ -973,58 +973,58 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
describe '#foreign_key_exists?' do
before do
key = ActiveRecord::ConnectionAdapters::ForeignKeyDefinition.new(
:projects, :users,
{
column: :non_standard_id,
name: :fk_projects_users_non_standard_id,
on_delete: :cascade,
primary_key: :id
}
)
allow(model).to receive(:foreign_keys).with(:projects).and_return([key])
model.connection.execute(<<~SQL)
create table referenced (
id bigserial primary key not null
);
create table referencing (
id bigserial primary key not null,
non_standard_id bigint not null,
constraint fk_referenced foreign key (non_standard_id) references referenced(id) on delete cascade
);
SQL
end
shared_examples_for 'foreign key checks' do
it 'finds existing foreign keys by column' do
expect(model.foreign_key_exists?(:projects, target_table, column: :non_standard_id)).to be_truthy
expect(model.foreign_key_exists?(:referencing, target_table, column: :non_standard_id)).to be_truthy
end
it 'finds existing foreign keys by name' do
expect(model.foreign_key_exists?(:projects, target_table, name: :fk_projects_users_non_standard_id)).to be_truthy
expect(model.foreign_key_exists?(:referencing, target_table, name: :fk_referenced)).to be_truthy
end
it 'finds existing foreign_keys by name and column' do
expect(model.foreign_key_exists?(:projects, target_table, name: :fk_projects_users_non_standard_id, column: :non_standard_id)).to be_truthy
expect(model.foreign_key_exists?(:referencing, target_table, name: :fk_referenced, column: :non_standard_id)).to be_truthy
end
it 'finds existing foreign_keys by name, column and on_delete' do
expect(model.foreign_key_exists?(:projects, target_table, name: :fk_projects_users_non_standard_id, column: :non_standard_id, on_delete: :cascade)).to be_truthy
expect(model.foreign_key_exists?(:referencing, target_table, name: :fk_referenced, column: :non_standard_id, on_delete: :cascade)).to be_truthy
end
it 'finds existing foreign keys by target table only' do
expect(model.foreign_key_exists?(:projects, target_table)).to be_truthy
expect(model.foreign_key_exists?(:referencing, target_table)).to be_truthy
end
it 'compares by column name if given' do
expect(model.foreign_key_exists?(:projects, target_table, column: :user_id)).to be_falsey
expect(model.foreign_key_exists?(:referencing, target_table, column: :user_id)).to be_falsey
end
it 'compares by target column name if given' do
expect(model.foreign_key_exists?(:projects, target_table, primary_key: :user_id)).to be_falsey
expect(model.foreign_key_exists?(:projects, target_table, primary_key: :id)).to be_truthy
expect(model.foreign_key_exists?(:referencing, target_table, primary_key: :user_id)).to be_falsey
expect(model.foreign_key_exists?(:referencing, target_table, primary_key: :id)).to be_truthy
end
it 'compares by foreign key name if given' do
expect(model.foreign_key_exists?(:projects, target_table, name: :non_existent_foreign_key_name)).to be_falsey
expect(model.foreign_key_exists?(:referencing, target_table, name: :non_existent_foreign_key_name)).to be_falsey
end
it 'compares by foreign key name and column if given' do
expect(model.foreign_key_exists?(:projects, target_table, name: :non_existent_foreign_key_name, column: :non_standard_id)).to be_falsey
expect(model.foreign_key_exists?(:referencing, target_table, name: :non_existent_foreign_key_name, column: :non_standard_id)).to be_falsey
end
it 'compares by foreign key name, column and on_delete if given' do
expect(model.foreign_key_exists?(:projects, target_table, name: :fk_projects_users_non_standard_id, column: :non_standard_id, on_delete: :nullify)).to be_falsey
expect(model.foreign_key_exists?(:referencing, target_table, name: :fk_referenced, column: :non_standard_id, on_delete: :nullify)).to be_falsey
end
end
@ -1035,7 +1035,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
end
context 'specifying a target table' do
let(:target_table) { :users }
let(:target_table) { :referenced }
it_behaves_like 'foreign key checks'
end
@ -1044,59 +1044,66 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
expect(model.foreign_key_exists?(:projects, :other_table)).to be_falsey
end
it 'raises an error if an invalid on_delete is specified' do
# The correct on_delete key is "nullify"
expect { model.foreign_key_exists?(:referenced, on_delete: :set_null) }.to raise_error(ArgumentError)
end
context 'with foreign key using multiple columns' do
before do
key = ActiveRecord::ConnectionAdapters::ForeignKeyDefinition.new(
:projects, :users,
{
column: [:partition_number, :id],
name: :fk_projects_users_partition_number_id,
on_delete: :cascade,
primary_key: [:partition_number, :id]
}
)
allow(model).to receive(:foreign_keys).with(:projects).and_return([key])
model.connection.execute(<<~SQL)
create table p_referenced (
id bigserial not null,
partition_number bigint not null default 100,
primary key (partition_number, id)
);
create table p_referencing (
id bigserial primary key not null,
partition_number bigint not null,
constraint fk_partitioning foreign key (partition_number, id) references p_referenced(partition_number, id) on delete cascade
);
SQL
end
it 'finds existing foreign keys by columns' do
expect(model.foreign_key_exists?(:projects, :users, column: [:partition_number, :id])).to be_truthy
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, column: [:partition_number, :id])).to be_truthy
end
it 'finds existing foreign keys by name' do
expect(model.foreign_key_exists?(:projects, :users, name: :fk_projects_users_partition_number_id)).to be_truthy
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :fk_partitioning)).to be_truthy
end
it 'finds existing foreign_keys by name and column' do
expect(model.foreign_key_exists?(:projects, :users, name: :fk_projects_users_partition_number_id, column: [:partition_number, :id])).to be_truthy
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :fk_partitioning, column: [:partition_number, :id])).to be_truthy
end
it 'finds existing foreign_keys by name, column and on_delete' do
expect(model.foreign_key_exists?(:projects, :users, name: :fk_projects_users_partition_number_id, column: [:partition_number, :id], on_delete: :cascade)).to be_truthy
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :fk_partitioning, column: [:partition_number, :id], on_delete: :cascade)).to be_truthy
end
it 'finds existing foreign keys by target table only' do
expect(model.foreign_key_exists?(:projects, :users)).to be_truthy
expect(model.foreign_key_exists?(:p_referencing, :p_referenced)).to be_truthy
end
it 'compares by column name if given' do
expect(model.foreign_key_exists?(:projects, :users, column: :id)).to be_falsey
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, column: :id)).to be_falsey
end
it 'compares by target column name if given' do
expect(model.foreign_key_exists?(:projects, :users, primary_key: :user_id)).to be_falsey
expect(model.foreign_key_exists?(:projects, :users, primary_key: [:partition_number, :id])).to be_truthy
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, primary_key: :user_id)).to be_falsey
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, primary_key: [:partition_number, :id])).to be_truthy
end
it 'compares by foreign key name if given' do
expect(model.foreign_key_exists?(:projects, :users, name: :non_existent_foreign_key_name)).to be_falsey
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :non_existent_foreign_key_name)).to be_falsey
end
it 'compares by foreign key name and column if given' do
expect(model.foreign_key_exists?(:projects, :users, name: :non_existent_foreign_key_name, column: [:partition_number, :id])).to be_falsey
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :non_existent_foreign_key_name, column: [:partition_number, :id])).to be_falsey
end
it 'compares by foreign key name, column and on_delete if given' do
expect(model.foreign_key_exists?(:projects, :users, name: :fk_projects_users_partition_number_id, column: [:partition_number, :id], on_delete: :nullify)).to be_falsey
expect(model.foreign_key_exists?(:p_referencing, :p_referenced, name: :fk_partitioning, column: [:partition_number, :id], on_delete: :nullify)).to be_falsey
end
end
end

77
spec/lib/gitlab/database/postgres_foreign_key_spec.rb
View File

@ -43,6 +43,14 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ
end
end
describe '#by_referenced_table_name' do
it 'finds the foreign keys for the referenced table' do
expected = described_class.find_by!(name: 'fk_constrained_to_referenced')
expect(described_class.by_referenced_table_name('referenced_table')).to contain_exactly(expected)
end
end
describe '#by_constrained_table_identifier' do
it 'throws an error when the identifier name is not fully qualified' do
expect { described_class.by_constrained_table_identifier('constrained_table') }.to raise_error(ArgumentError, /not fully qualified/)
@ -55,9 +63,25 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ
end
end
describe '#by_constrained_table_name' do
it 'finds the foreign keys for the constrained table' do
expected = described_class.where(name: %w[fk_constrained_to_referenced fk_constrained_to_other_referenced]).to_a
expect(described_class.by_constrained_table_name('constrained_table')).to match_array(expected)
end
end
describe '#by_name' do
it 'finds foreign keys by name' do
expect(described_class.by_name('fk_constrained_to_referenced').pluck(:name)).to contain_exactly('fk_constrained_to_referenced')
end
end
context 'when finding columns for foreign keys' do
using RSpec::Parameterized::TableSyntax
let(:fks) { described_class.by_constrained_table_name('constrained_table') }
where(:fk, :expected_constrained, :expected_referenced) do
lazy { described_class.find_by(name: 'fk_constrained_to_referenced') } | %w[referenced_table_id referenced_table_id_b] | %w[id id_b]
lazy { described_class.find_by(name: 'fk_constrained_to_other_referenced') } | %w[other_referenced_table_id] | %w[id]
@ -71,23 +95,70 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model, feature_categ
it 'finds the correct referenced column names' do
expect(fk.referenced_columns).to eq(expected_referenced)
end
describe '#by_constrained_columns' do
it 'finds the correct foreign key' do
expect(fks.by_constrained_columns(expected_constrained)).to contain_exactly(fk)
end
end
describe '#by_referenced_columns' do
it 'finds the correct foreign key' do
expect(fks.by_referenced_columns(expected_referenced)).to contain_exactly(fk)
end
end
end
end
describe '#on_delete_action' do
before do
ApplicationRecord.connection.execute(<<~SQL)
create table public.referenced_table_all_on_delete_actions (
id bigserial primary key not null
);
create table public.constrained_table_all_on_delete_actions (
id bigserial primary key not null,
ref_id_no_action bigint not null constraint fk_no_action references referenced_table_all_on_delete_actions(id),
ref_id_restrict bigint not null constraint fk_restrict references referenced_table_all_on_delete_actions(id) on delete restrict,
ref_id_nullify bigint not null constraint fk_nullify references referenced_table_all_on_delete_actions(id) on delete set null,
ref_id_cascade bigint not null constraint fk_cascade references referenced_table_all_on_delete_actions(id) on delete cascade,
ref_id_set_default bigint not null constraint fk_set_default references referenced_table_all_on_delete_actions(id) on delete set default
)
SQL
end
let(:fks) { described_class.by_constrained_table_name('constrained_table_all_on_delete_actions') }
context 'with an invalid on_delete_action' do
it 'raises an error' do
# the correct value is :nullify, not :set_null
expect { fks.by_on_delete_action(:set_null) }.to raise_error(ArgumentError)
end
end
where(:fk_name, :expected_on_delete_action) do
[
%w[fk_constrained_to_referenced restrict],
%w[fk_constrained_to_other_referenced no_action]
%w[fk_no_action no_action],
%w[fk_restrict restrict],
%w[fk_nullify nullify],
%w[fk_cascade cascade],
%w[fk_set_default set_default]
]
end
with_them do
subject(:fk) { described_class.find_by(name: fk_name) }
subject(:fk) { fks.find_by(name: fk_name) }
it 'has the appropriate on delete action' do
expect(fk.on_delete_action).to eq(expected_on_delete_action)
end
describe '#by_on_delete_action' do
it 'finds the key by on delete action' do
expect(fks.by_on_delete_action(expected_on_delete_action)).to contain_exactly(fk)
end
end
end
end

18
spec/migrations/queue_backfill_admin_mode_scope_for_personal_access_tokens_spec.rb Normal file
View File

@ -0,0 +1,18 @@
# frozen_string_literal: true
require 'spec_helper'
require_migration!
RSpec.describe QueueBackfillAdminModeScopeForPersonalAccessTokens,
feature_category: :authentication_and_authorization do
describe '#up' do
it 'schedules background migration' do
migrate!
expect(described_class::MIGRATION).to have_scheduled_batched_migration(
table_name: :personal_access_tokens,
column_name: :id,
interval: described_class::DELAY_INTERVAL)
end
end
end

15
spec/models/abuse_report_spec.rb
View File

@ -20,6 +20,11 @@ RSpec.describe AbuseReport, feature_category: :insider_threat do
end
describe 'validations' do
let(:http) { 'http://gitlab.com' }
let(:https) { 'https://gitlab.com' }
let(:ftp) { 'ftp://example.com' }
let(:javascript) { 'javascript:alert(window.opener.document.location)' }
it { is_expected.to validate_presence_of(:reporter) }
it { is_expected.to validate_presence_of(:user) }
it { is_expected.to validate_presence_of(:message) }
@ -28,8 +33,16 @@ RSpec.describe AbuseReport, feature_category: :insider_threat do
it do
is_expected.to validate_uniqueness_of(:user_id)
.scoped_to(:reporter_id)
.with_message('You have already reported this user')
.with_message('has already been reported for abuse')
end
it { is_expected.to validate_length_of(:reported_from_url).is_at_most(512).allow_blank }
it { is_expected.to allow_value(http).for(:reported_from_url) }
it { is_expected.to allow_value(https).for(:reported_from_url) }
it { is_expected.not_to allow_value(ftp).for(:reported_from_url) }
it { is_expected.not_to allow_value(javascript).for(:reported_from_url) }
it { is_expected.to allow_value('http://localhost:9000').for(:reported_from_url) }
it { is_expected.to allow_value('https://gitlab.com').for(:reported_from_url) }
end
describe '#remove_user' do

2
spec/models/integrations/apple_app_store_spec.rb
View File

@ -29,7 +29,7 @@ RSpec.describe Integrations::AppleAppStore, feature_category: :mobile_devops do
describe '#fields' do
it 'returns custom fields' do
expect(apple_app_store_integration.fields.pluck(:name)).to eq(%w[app_store_issuer_id app_store_key_id
app_store_private_key])
app_store_private_key])
end
end

31
spec/models/personal_access_token_spec.rb
View File

@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe PersonalAccessToken do
RSpec.describe PersonalAccessToken, feature_category: :authentication_and_authorization do
subject { described_class }
describe '.build' do
@ -210,6 +210,12 @@ RSpec.describe PersonalAccessToken do
expect(personal_access_token).to be_valid
end
it "allows creating a token with `admin_mode` scope" do
personal_access_token.scopes = [:api, :admin_mode]
expect(personal_access_token).to be_valid
end
context 'when registry is disabled' do
before do
stub_container_registry_config(enabled: false)
@ -340,4 +346,27 @@ RSpec.describe PersonalAccessToken do
end
end
end
# During the implementation of Admin Mode for API, tokens of
# administrators should automatically get the `admin_mode` scope as well
# See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
describe '`admin_mode scope' do
subject { create(:personal_access_token, user: user, scopes: ['api']) }
context 'with administrator user' do
let_it_be(:user) { create(:user, :admin) }
it 'adds `admin_mode` scope before created' do
expect(subject.scopes).to contain_exactly('api', 'admin_mode')
end
end
context 'with normal user' do
let_it_be(:user) { create(:user) }
it 'does not add `admin_mode` scope before created' do
expect(subject.scopes).to contain_exactly('api')
end
end
end
end

22
spec/requests/abuse_reports_controller_spec.rb
View File

@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe AbuseReportsController, feature_category: :users do
RSpec.describe AbuseReportsController, feature_category: :insider_threat do
let(:reporter) { create(:user) }
let(:user) { create(:user) }
let(:attrs) do
@ -16,6 +16,18 @@ RSpec.describe AbuseReportsController, feature_category: :users do
end
describe 'GET new' do
let(:ref_url) { 'http://example.com' }
it 'sets the instance variables' do
get new_abuse_report_path(user_id: user.id, ref_url: ref_url)
expect(assigns(:abuse_report)).to be_kind_of(AbuseReport)
expect(assigns(:abuse_report)).to have_attributes(
user_id: user.id,
reported_from_url: ref_url
)
end
context 'when the user has already been deleted' do
it 'redirects the reporter to root_path' do
user_id = user.id
@ -47,7 +59,9 @@ RSpec.describe AbuseReportsController, feature_category: :users do
context 'when user is reported for abuse' do
let(:ref_url) { 'http://example.com' }
let(:request_params) { { user_id: user.id, abuse_report: { category: abuse_category }, ref_url: ref_url } }
let(:request_params) do
{ user_id: user.id, abuse_report: { category: abuse_category, reported_from_url: ref_url } }
end
it 'renders new template' do
subject
@ -62,9 +76,9 @@ RSpec.describe AbuseReportsController, feature_category: :users do
expect(assigns(:abuse_report)).to be_kind_of(AbuseReport)
expect(assigns(:abuse_report)).to have_attributes(
user_id: user.id,
category: abuse_category
category: abuse_category,
reported_from_url: ref_url
)
expect(assigns(:ref_url)).to eq(ref_url)
end
end

12
spec/requests/api/internal/base_spec.rb
View File

@ -453,16 +453,10 @@ RSpec.describe API::Internal::Base, feature_category: :authentication_and_author
expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.')
end
context 'when rate_limit_gitlab_shell feature flag is disabled' do
before do
stub_feature_flags(rate_limit_gitlab_shell: false)
end
it 'is not throttled by rate limiter' do
expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?)
it 'is not throttled by rate limiter' do
expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?)
subject
end
subject
end
context 'when the IP is in a trusted range' do

12
spec/requests/api/projects_spec.rb
View File

@ -1169,7 +1169,7 @@ RSpec.describe API::Projects do
expect(response).to have_gitlab_http_status(:bad_request)
end
it "assigns attributes to project", :aggregate_failures do
it 'assigns attributes to project', :aggregate_failures do
project = attributes_for(:project, {
path: 'camelCasePath',
issues_enabled: false,
@ -1198,6 +1198,11 @@ RSpec.describe API::Projects do
attrs[:feature_flags_access_level] = 'disabled'
attrs[:infrastructure_access_level] = 'disabled'
attrs[:monitor_access_level] = 'disabled'
attrs[:snippets_access_level] = 'disabled'
attrs[:wiki_access_level] = 'disabled'
attrs[:builds_access_level] = 'disabled'
attrs[:merge_requests_access_level] = 'disabled'
attrs[:issues_access_level] = 'disabled'
end
post api('/projects', user), params: project
@ -1228,6 +1233,11 @@ RSpec.describe API::Projects do
expect(project.project_feature.feature_flags_access_level).to eq(ProjectFeature::DISABLED)
expect(project.project_feature.infrastructure_access_level).to eq(ProjectFeature::DISABLED)
expect(project.project_feature.monitor_access_level).to eq(ProjectFeature::DISABLED)
expect(project.project_feature.wiki_access_level).to eq(ProjectFeature::DISABLED)
expect(project.project_feature.builds_access_level).to eq(ProjectFeature::DISABLED)
expect(project.project_feature.merge_requests_access_level).to eq(ProjectFeature::DISABLED)
expect(project.project_feature.issues_access_level).to eq(ProjectFeature::DISABLED)
expect(project.project_feature.snippets_access_level).to eq(ProjectFeature::DISABLED)
end
it 'assigns container_registry_enabled to project', :aggregate_failures do

12
spec/requests/api/repositories_spec.rb
View File

@ -353,15 +353,9 @@ RSpec.describe API::Repositories, feature_category: :source_code_management do
expect(response).to have_gitlab_http_status(:too_many_requests)
end
context "when hotlinking detection is enabled" do
before do
stub_feature_flags(repository_archive_hotlinking_interception: true)
end
it_behaves_like "hotlink interceptor" do
let(:http_request) do
get api(route, current_user), headers: headers
end
it_behaves_like "hotlink interceptor" do
let(:http_request) do
get api(route, current_user), headers: headers
end
end
end

2
spec/support/shared_examples/features/reportable_note_shared_examples.rb
View File

@ -36,7 +36,7 @@ RSpec.shared_examples 'reportable note' do |type|
dropdown.click_link('Report abuse to administrator')
expect(find('#user_name')['value']).to match(note.author.username)
expect(find('#abuse_report_message')['value']).to match(noteable_note_url(note))
expect(find('#abuse_report_reported_from_url')['value']).to match(noteable_note_url(note))
end
def open_dropdown(dropdown)

2
workhorse/go.mod
View File

@ -17,7 +17,7 @@ require (
github.com/gorilla/websocket v1.5.0
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
github.com/johannesboyne/gofakes3 v0.0.0-20230104192229-1065b17a924f
github.com/johannesboyne/gofakes3 v0.0.0-20230108161031-df26ca44a1e9
github.com/jpillora/backoff v1.0.0
github.com/mitchellh/copystructure v1.2.0
github.com/prometheus/client_golang v1.14.0

4
workhorse/go.sum
View File

@ -981,8 +981,8 @@ github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHW
github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8=
github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U=
github.com/joefitzgerald/rainbow-reporter v0.1.0/go.mod h1:481CNgqmVHQZzdIbN52CupLJyoVwB10FQ/IQlF1pdL8=
github.com/johannesboyne/gofakes3 v0.0.0-20230104192229-1065b17a924f h1:m/Y0+A6QxQ00DPR0XmpOzoCmBOWO9J4XqBGiHonbMj8=
github.com/johannesboyne/gofakes3 v0.0.0-20230104192229-1065b17a924f/go.mod h1:Cnosl0cRZIfKjTMuH49sQog2LeNsU5Hf4WnPIDWIDV0=
github.com/johannesboyne/gofakes3 v0.0.0-20230108161031-df26ca44a1e9 h1:PqhUbDge60cL99naOP9m3W0MiQtWc5kwteQQ9oU36PA=
github.com/johannesboyne/gofakes3 v0.0.0-20230108161031-df26ca44a1e9/go.mod h1:Cnosl0cRZIfKjTMuH49sQog2LeNsU5Hf4WnPIDWIDV0=
github.com/joho/godotenv v1.3.0/go.mod h1:7hK45KPybAkOC6peb+G5yklZfMxEjkZhHbwpqxOKXbg=
github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo=
github.com/jonboulle/clockwork v0.2.2/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8=