diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 46414b49f37eb5c32689608647e77dd10aee6d75..efdad22fa54c54fd5cd47259a2614334881ae80d 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -338,6 +338,8 @@ def visible_attributes :version_check_enabled, :web_ide_clientside_preview_enabled, :diff_max_patch_bytes, + :diff_max_files, + :diff_max_lines, :commit_email_hostname, :protected_ci_variables, :local_markdown_version, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 65800e40d6c8224110760943f3432004f3eccd59..f8047ed9b7848bfbc5b37ebeea8989156f3de285 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -273,6 +273,18 @@ def self.kroki_formats_attributes greater_than_or_equal_to: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, less_than_or_equal_to: Gitlab::Git::Diff::MAX_PATCH_BYTES_UPPER_BOUND } + validates :diff_max_files, + presence: true, + numericality: { only_integer: true, + greater_than_or_equal_to: Commit::DEFAULT_MAX_DIFF_FILES_SETTING, + less_than_or_equal_to: Commit::MAX_DIFF_FILES_SETTING_UPPER_BOUND } + + validates :diff_max_lines, + presence: true, + numericality: { only_integer: true, + greater_than_or_equal_to: Commit::DEFAULT_MAX_DIFF_LINES_SETTING, + less_than_or_equal_to: Commit::MAX_DIFF_LINES_SETTING_UPPER_BOUND } + validates :user_default_internal_regex, js_regex: true, allow_nil: true validates :personal_access_token_prefix, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index bf9df3b9efc119f8d7acf8aa1a5b7d17bcc93b4d..b613e698471f8e5273c0915f944fdfa110a8450b 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -60,6 +60,8 @@ def defaults default_projects_limit: Settings.gitlab['default_projects_limit'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, + diff_max_files: Commit::DEFAULT_MAX_DIFF_FILES_SETTING, + diff_max_lines: Commit::DEFAULT_MAX_DIFF_LINES_SETTING, disable_feed_token: false, disabled_oauth_sign_in_sources: [], dns_rebinding_protection_enabled: true, diff --git a/app/models/commit.rb b/app/models/commit.rb index 23c1dffcc63c1d7eb5406a1c0b8946952438caaa..a1ed5eb9ab95001f303cde8192882a94759e4860 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -33,6 +33,12 @@ class Commit # Used by GFM to match and present link extensions on node texts and hrefs. LINK_EXTENSION_PATTERN = /(patch)/.freeze + DEFAULT_MAX_DIFF_LINES_SETTING = 50_000 + DEFAULT_MAX_DIFF_FILES_SETTING = 1_000 + MAX_DIFF_LINES_SETTING_UPPER_BOUND = 100_000 + MAX_DIFF_FILES_SETTING_UPPER_BOUND = 3_000 + DIFF_SAFE_LIMIT_FACTOR = 10 + cache_markdown_field :title, pipeline: :single_line cache_markdown_field :full_title, pipeline: :single_line, limit: 1.kilobyte cache_markdown_field :description, pipeline: :commit_description, limit: 1.megabyte @@ -78,20 +84,24 @@ def truncate_sha(sha) end def diff_safe_lines(project: nil) - Gitlab::Git::DiffCollection.default_limits(project: project)[:max_lines] + diff_safe_max_lines(project: project) end - def diff_hard_limit_files(project: nil) + def diff_max_files(project: nil) if Feature.enabled?(:increased_diff_limits, project) 3000 + elsif Feature.enabled?(:configurable_diff_limits, project) + Gitlab::CurrentSettings.diff_max_files else 1000 end end - def diff_hard_limit_lines(project: nil) + def diff_max_lines(project: nil) if Feature.enabled?(:increased_diff_limits, project) 100000 + elsif Feature.enabled?(:configurable_diff_limits, project) + Gitlab::CurrentSettings.diff_max_lines else 50000 end @@ -99,11 +109,19 @@ def diff_hard_limit_lines(project: nil) def max_diff_options(project: nil) { - max_files: diff_hard_limit_files(project: project), - max_lines: diff_hard_limit_lines(project: project) + max_files: diff_max_files(project: project), + max_lines: diff_max_lines(project: project) } end + def diff_safe_max_files(project: nil) + diff_max_files(project: project) / DIFF_SAFE_LIMIT_FACTOR + end + + def diff_safe_max_lines(project: nil) + diff_max_lines(project: project) / DIFF_SAFE_LIMIT_FACTOR + end + def from_hash(hash, container) raw_commit = Gitlab::Git::Commit.new(container.repository.raw, hash) new(raw_commit, container) diff --git a/app/views/admin/application_settings/_diff_limits.html.haml b/app/views/admin/application_settings/_diff_limits.html.haml index 5351ac5abd1e1cb471f7085fd81fc8730168527e..6a51d2e39d40a60a3889b5225525070547d17795 100644 --- a/app/views/admin/application_settings/_diff_limits.html.haml +++ b/app/views/admin/application_settings/_diff_limits.html.haml @@ -3,11 +3,30 @@ %fieldset .form-group - = f.label :diff_max_patch_bytes, _('Maximum diff patch size in bytes'), class: 'label-light' + = f.label :diff_max_patch_bytes, _('Maximum diff patch size (Bytes)'), class: 'label-light' = f.number_field :diff_max_patch_bytes, class: 'form-control gl-form-input' %span.form-text.text-muted - = _("Collapse diffs larger than this size, and show a 'too large' message instead.") + = _("Diff files surpassing this limit will be presented as 'too large' and won't be expandable.") = link_to sprite_icon('question-o'), - help_page_path('user/admin_area/diff_limits') + help_page_path('user/admin_area/diff_limits', + anchor: 'diff-limits-administration') + + = f.label :diff_max_files, _('Maximum files in a diff'), class: 'label-light' + = f.number_field :diff_max_files, class: 'form-control gl-form-input' + %span.form-text.text-muted + = _("Diff files surpassing this limit will be presented as 'too large' and won't be expandable.") + + = link_to sprite_icon('question-o'), + help_page_path('user/admin_area/diff_limits', + anchor: 'diff-limits-administration') + + = f.label :diff_max_lines, _('Maximum lines in a diff'), class: 'label-light' + = f.number_field :diff_max_lines, class: 'form-control gl-form-input' + %span.form-text.text-muted + = _("Diff files surpassing this limit will be presented as 'too large' and won't be expandable.") + + = link_to sprite_icon('question-o'), + help_page_path('user/admin_area/diff_limits', + anchor: 'diff-limits-administration') = f.submit _('Save changes'), class: 'gl-button btn btn-confirm' diff --git a/config/feature_flags/development/configurable_diff_limits.yml b/config/feature_flags/development/configurable_diff_limits.yml new file mode 100644 index 0000000000000000000000000000000000000000..e73d45fac65edce94413d2e020021db67a554073 --- /dev/null +++ b/config/feature_flags/development/configurable_diff_limits.yml @@ -0,0 +1,8 @@ +--- +name: configurable_diff_limits +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56722 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332194 +milestone: '14.0' +type: development +group: group::code review +default_enabled: false diff --git a/db/migrate/20210527133919_add_diff_max_lines_to_application_settings.rb b/db/migrate/20210527133919_add_diff_max_lines_to_application_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..9c1cd94dbaa0e324c99fb98eef25fd72ef5d6c4f --- /dev/null +++ b/db/migrate/20210527133919_add_diff_max_lines_to_application_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddDiffMaxLinesToApplicationSettings < ActiveRecord::Migration[6.0] + def change + add_column(:application_settings, + :diff_max_lines, + :integer, + default: 50000, + null: false) + end +end diff --git a/db/migrate/20210527134019_add_diff_max_files_to_application_settings.rb b/db/migrate/20210527134019_add_diff_max_files_to_application_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..60b1f74cfd0129cb8f9b276891fa7392369df44d --- /dev/null +++ b/db/migrate/20210527134019_add_diff_max_files_to_application_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddDiffMaxFilesToApplicationSettings < ActiveRecord::Migration[6.0] + def change + add_column(:application_settings, + :diff_max_files, + :integer, + default: 1000, + null: false) + end +end diff --git a/db/schema_migrations/20210527133919 b/db/schema_migrations/20210527133919 new file mode 100644 index 0000000000000000000000000000000000000000..559860de55dfed0ff476938ced5bad7ee547c4b6 --- /dev/null +++ b/db/schema_migrations/20210527133919 @@ -0,0 +1 @@ +aaf5936c945451fa98df7c21ab34c9aa7190dcf301f536c259e5b1fe54407f36 \ No newline at end of file diff --git a/db/schema_migrations/20210527134019 b/db/schema_migrations/20210527134019 new file mode 100644 index 0000000000000000000000000000000000000000..de757dd355e1a002412691d5834a57eeaf0c6e34 --- /dev/null +++ b/db/schema_migrations/20210527134019 @@ -0,0 +1 @@ +ac4522ee51d4a4cda317b680c16be3d9ef3e1619bba80c26aefe8d5dc70f013c \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3c96987bb5cbb85f353b1b3eedf6223bed8d3ae2..e3ac9891ad7f046602e300450483e648b8d4cae7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9517,6 +9517,8 @@ CREATE TABLE application_settings ( elasticsearch_username text, encrypted_elasticsearch_password bytea, encrypted_elasticsearch_password_iv bytea, + diff_max_lines integer DEFAULT 50000 NOT NULL, + diff_max_files integer DEFAULT 1000 NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)), diff --git a/doc/api/settings.md b/doc/api/settings.md index 406688732feaa45383d97136559b14bdb740a21f..3739eabca6f1af39af20bfc3f9f5299c0fba4a8d 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -251,7 +251,9 @@ listed in the descriptions of the relevant settings. | `default_projects_limit` | integer | no | Project limit per user. Default is `100000`. | | `default_snippet_visibility` | string | no | What visibility level new snippets receive. Can take `private`, `internal` and `public` as a parameter. Default is `private`. | | `deletion_adjourned_period` | integer | no | **(PREMIUM SELF)** The number of days to wait before deleting a project or group that is marked for deletion. Value must be between 0 and 90. -| `diff_max_patch_bytes` | integer | no | Maximum diff patch size (Bytes). | +| `diff_max_patch_bytes` | integer | no | Maximum [diff patch size](../user/admin_area/diff_limits.md), in bytes. | +| `diff_max_files` | integer | no | Maximum [files in a diff](../user/admin_area/diff_limits.md). | +| `diff_max_lines` | integer | no | Maximum [lines in a diff](../user/admin_area/diff_limits.md). | | `disable_feed_token` | boolean | no | Disable display of RSS/Atom and calendar feed tokens ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/231493) in GitLab 13.7) | | `disabled_oauth_sign_in_sources` | array of strings | no | Disabled OAuth sign-in sources. | | `dns_rebinding_protection_enabled` | boolean | no | Enforce DNS rebinding attack protection. | diff --git a/doc/user/admin_area/diff_limits.md b/doc/user/admin_area/diff_limits.md index 88d24680500bff6f36797cec653f015ecbdae931..881f2e6a93bc8512f2cf57d01715355058fceb08 100644 --- a/doc/user/admin_area/diff_limits.md +++ b/doc/user/admin_area/diff_limits.md @@ -12,27 +12,31 @@ You can set a maximum size for display of diff files (patches). For details about diff files, [view changes between files](../project/merge_requests/changes.md). Read more about the [built-in limits for merge requests and diffs](../../administration/instance_limits.md#merge-requests). -## Maximum diff patch size +## Configure diff limits -Diff files which exceed this value are presented as 'too large' and cannot -be expandable. Instead of an expandable view, a link to the blob view is -shown. +WARNING: +These settings are experimental. An increased maximum increases resource +consumption of your instance. Keep this in mind when adjusting the maximum. + +To speed the loading time of merge request views and branch comparison views +on your instance, you can configure three instance-level maximum values for diffs: + +- **Maximum diff patch size**: The total size, in bytes, of the entire diff. +- **Maximum diff files**: The total number of files changed in a diff. +- **Maximum diff files**: The total number of files changed in a diff. The default value is 1000. +- **Maximum diff lines**: The total number of lines changed in a diff. The default value is 50,000. -Patches greater than 10% of this size are automatically collapsed, and a -link to expand the diff is presented. -This affects merge requests and branch comparison views. +When a diff reaches 10% of any of these values, the files are shown in a +collapsed view, with a link to expand the diff. Diffs that exceed any of the +set values are presented as **Too large** are cannot be expanded in the UI. -To set the maximum diff patch size: +To configure these values: 1. Go to the Admin Area (**{admin}**) and select **Settings > General**. 1. Expand **Diff limits**. 1. Enter a value for **Maximum diff patch size**, measured in bytes. 1. Select **Save changes**. -WARNING: -This setting is experimental. An increased maximum increases resource -consumption of your instance. Keep this in mind when adjusting the maximum. -