From 1a54a22498c83026e61d30d36c9c599938664454 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 2 Mar 2022 03:13:05 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- GITALY_SERVER_VERSION | 2 +- .../ide/components/file_templates/bar.vue | 4 +- .../components/file_templates/dropdown.vue | 2 +- .../serverless/functions_controller.rb | 5 + .../concerns/blocks_json_serialization.rb | 18 -- .../concerns/blocks_unsafe_serialization.rb | 32 +++ app/models/project.rb | 5 + app/models/project_import_data.rb | 7 +- app/models/user.rb | 2 +- .../abuse_reports/_abuse_report.html.haml | 4 +- .../application_settings/_signin.html.haml | 8 +- .../block_project_serialization.yml | 8 + .../ops/deprecated_serverless.yml | 8 + doc/administration/operations/puma.md | 225 +++++++++--------- doc/administration/troubleshooting/debug.md | 2 +- doc/development/permissions.md | 17 ++ doc/install/requirements.md | 2 +- .../admin_area/settings/gitaly_timeouts.md | 2 +- doc/user/gitlab_com/index.md | 2 +- doc/user/group/index.md | 4 +- doc/user/index.md | 13 +- .../merge_requests/approvals/settings.md | 4 +- lib/serializers/unsafe_json.rb | 13 + .../projects/menus/infrastructure_menu.rb | 2 +- locale/gitlab.pot | 12 +- qa/qa/page/project/web_ide/edit.rb | 2 +- qa/qa/tools/delete_test_users.rb | 10 +- .../boards/lists_controller_spec.rb | 4 +- .../serverless/functions_controller_spec.rb | 23 ++ ...ssuable_reference_expansion_filter_spec.rb | 2 +- spec/lib/serializers/unsafe_json_spec.rb | 27 +++ .../menus/infrastructure_menu_spec.rb | 8 + .../blocks_json_serialization_spec.rb | 22 -- .../blocks_unsafe_serialization_spec.rb | 17 ++ spec/models/project_spec.rb | 14 ++ spec/models/user_spec.rb | 2 +- ...ks_unsafe_serialization_shared_examples.rb | 26 ++ .../note_entity_shared_examples.rb | 23 ++ 38 files changed, 392 insertions(+), 191 deletions(-) delete mode 100644 app/models/concerns/blocks_json_serialization.rb create mode 100644 app/models/concerns/blocks_unsafe_serialization.rb create mode 100644 config/feature_flags/development/block_project_serialization.yml create mode 100644 config/feature_flags/ops/deprecated_serverless.yml create mode 100644 lib/serializers/unsafe_json.rb create mode 100644 spec/lib/serializers/unsafe_json_spec.rb delete mode 100644 spec/models/concerns/blocks_json_serialization_spec.rb create mode 100644 spec/models/concerns/blocks_unsafe_serialization_spec.rb create mode 100644 spec/support/shared_examples/blocks_unsafe_serialization_shared_examples.rb diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 0565b7b1a7b..d3bd732e4aa 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -416988ddd41d192114142a828eb039fac450d084 +4e5774f37ab8581cf0a988ffca97a6252078ddc8 diff --git a/app/assets/javascripts/ide/components/file_templates/bar.vue b/app/assets/javascripts/ide/components/file_templates/bar.vue index ced84898cb4..0921b5a5424 100644 --- a/app/assets/javascripts/ide/components/file_templates/bar.vue +++ b/app/assets/javascripts/ide/components/file_templates/bar.vue @@ -84,7 +84,7 @@ export default { > {{ $options.i18n.barLabel }}
diff --git a/app/assets/javascripts/ide/components/file_templates/dropdown.vue b/app/assets/javascripts/ide/components/file_templates/dropdown.vue index ec61e3374d7..e8b42ac9490 100644 --- a/app/assets/javascripts/ide/components/file_templates/dropdown.vue +++ b/app/assets/javascripts/ide/components/file_templates/dropdown.vue @@ -84,7 +84,7 @@ export default { v-model="search" :placeholder="__('Filter...')" type="search" - class="dropdown-input-field qa-dropdown-filter-input" + class="dropdown-input-field" />
diff --git a/app/controllers/projects/serverless/functions_controller.rb b/app/controllers/projects/serverless/functions_controller.rb index 3fc379a135a..b6f77a6d515 100644 --- a/app/controllers/projects/serverless/functions_controller.rb +++ b/app/controllers/projects/serverless/functions_controller.rb @@ -3,6 +3,7 @@ module Projects module Serverless class FunctionsController < Projects::ApplicationController + before_action :ensure_feature_enabled! before_action :authorize_read_cluster! feature_category :not_owned @@ -69,6 +70,10 @@ module Projects def serialize_function(function) Projects::Serverless::ServiceSerializer.new(current_user: @current_user, project: project).represent(function) end + + def ensure_feature_enabled! + render_404 unless Feature.enabled?(:deprecated_serverless, project, default_enabled: :yaml, type: :ops) + end end end end diff --git a/app/models/concerns/blocks_json_serialization.rb b/app/models/concerns/blocks_json_serialization.rb deleted file mode 100644 index 18c00532d78..00000000000 --- a/app/models/concerns/blocks_json_serialization.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -# Overrides `as_json` and `to_json` to raise an exception when called in order -# to prevent accidentally exposing attributes -# -# Not that would ever happen... but just in case. -module BlocksJsonSerialization - extend ActiveSupport::Concern - - JsonSerializationError = Class.new(StandardError) - - def to_json(*) - raise JsonSerializationError, - "JSON serialization has been disabled on #{self.class.name}" - end - - alias_method :as_json, :to_json -end diff --git a/app/models/concerns/blocks_unsafe_serialization.rb b/app/models/concerns/blocks_unsafe_serialization.rb new file mode 100644 index 00000000000..72adbe70f15 --- /dev/null +++ b/app/models/concerns/blocks_unsafe_serialization.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +# Overrides `#serializable_hash` to raise an exception when called without the `only` option +# in order to prevent accidentally exposing attributes. +# +# An `unsafe: true` option can also be passed in to bypass this check. +# +# `#serializable_hash` is used by ActiveModel serializers like `ActiveModel::Serializers::JSON` +# which overrides `#as_json` and `#to_json`. +# +module BlocksUnsafeSerialization + extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override + + UnsafeSerializationError = Class.new(StandardError) + + override :serializable_hash + def serializable_hash(options = nil) + return super if allow_serialization?(options) + + raise UnsafeSerializationError, + "Serialization has been disabled on #{self.class.name}" + end + + private + + def allow_serialization?(options = nil) + return false unless options + + !!(options[:only] || options[:unsafe]) + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 09ea18d295c..13b4a6a9c74 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -38,6 +38,7 @@ class Project < ApplicationRecord include GitlabRoutingHelper include BulkMemberAccessLoad include RunnerTokenExpirationInterval + include BlocksUnsafeSerialization extend Gitlab::Cache::RequestCache extend Gitlab::Utils::Override @@ -3047,6 +3048,10 @@ class Project < ApplicationRecord Projects::SyncEvent.enqueue_worker end end + + def allow_serialization?(options = nil) + Feature.disabled?(:block_project_serialization, self, default_enabled: :yaml) || super + end end Project.prepend_mod_with('Project') diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index d374ee120d1..3b514d5c5ff 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -14,7 +14,12 @@ class ProjectImportData < ApplicationRecord insecure_mode: true, algorithm: 'aes-256-cbc' - serialize :data, JSON # rubocop:disable Cop/ActiveRecordSerialize + # NOTE + # We are serializing a project as `data` in an "unsafe" way here + # because the credentials are necessary for a successful import. + # This is safe because the serialization is only going between rails + # and the database, never to any end users. + serialize :data, Serializers::UnsafeJson # rubocop:disable Cop/ActiveRecordSerialize validates :project, presence: true diff --git a/app/models/user.rb b/app/models/user.rb index 5322fe1142a..2e4f635b787 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -16,7 +16,7 @@ class User < ApplicationRecord include FeatureGate include CreatedAtFilterable include BulkMemberAccessLoad - include BlocksJsonSerialization + include BlocksUnsafeSerialization include WithUploads include OptionallySearch include FromUnion diff --git a/app/views/admin/abuse_reports/_abuse_report.html.haml b/app/views/admin/abuse_reports/_abuse_report.html.haml index dbfc7bf1046..00e5650b551 100644 --- a/app/views/admin/abuse_reports/_abuse_report.html.haml +++ b/app/views/admin/abuse_reports/_abuse_report.html.haml @@ -25,9 +25,9 @@ %td - if user = link_to _('Remove user & report'), admin_abuse_report_path(abuse_report, remove_user: true), - data: { confirm: _("USER %{user} WILL BE REMOVED! Are you sure?") % { user: user.name } }, remote: true, method: :delete, class: "gl-button btn btn-block btn-danger js-remove-tr" + data: { confirm: _("USER %{user} WILL BE REMOVED! Are you sure?") % { user: user.name }, confirm_btn_variant: "danger" }, aria: { label: _('Remove user & report') }, remote: true, method: :delete, class: "gl-button btn btn-block btn-danger js-remove-tr" - if user && !user.blocked? - = link_to _('Block user'), block_admin_user_path(user), data: {confirm: _('USER WILL BE BLOCKED! Are you sure?')}, method: :put, class: "gl-button btn btn-default btn-block" + = link_to _('Block user'), block_admin_user_path(user), data: { confirm: _('USER WILL BE BLOCKED! Are you sure?') }, aria: { label: _('Block user') }, method: :put, class: "gl-button btn btn-default btn-block" - else .gl-button.btn.btn-default.disabled.btn-block = _('Already blocked') diff --git a/app/views/admin/application_settings/_signin.html.haml b/app/views/admin/application_settings/_signin.html.haml index 156e7d3fb76..bce210d28d3 100644 --- a/app/views/admin/application_settings/_signin.html.haml +++ b/app/views/admin/application_settings/_signin.html.haml @@ -8,17 +8,17 @@ = f.label :password_authentication_enabled_for_web, class: 'form-check-label' do = _('Allow password authentication for the web interface') .form-text.text-muted - = _('When inactive, an external authentication provider must be used.') + = _('Clear this checkbox to use an external authentication provider instead.') .form-group .form-check = f.check_box :password_authentication_enabled_for_git, class: 'form-check-input' = f.label :password_authentication_enabled_for_git, class: 'form-check-label' do = _('Allow password authentication for Git over HTTP(S)') .form-text.text-muted - When inactive, a Personal Access Token - if Gitlab::Auth::Ldap::Config.enabled? - or LDAP password - must be used to authenticate. + = _('Clear this checkbox to use a personal access token or LDAP password instead.') + - else + = _('Clear this checkbox to use a personal access token instead.') - if omniauth_enabled? && button_based_providers.any? %fieldset.form-group %legend.gl-font-base.gl-mb-3.gl-border-none.gl-font-weight-bold= _('Enabled OAuth authentication sources') diff --git a/config/feature_flags/development/block_project_serialization.yml b/config/feature_flags/development/block_project_serialization.yml new file mode 100644 index 00000000000..c4ef7145fcb --- /dev/null +++ b/config/feature_flags/development/block_project_serialization.yml @@ -0,0 +1,8 @@ +--- +name: block_project_serialization +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/81900 +rollout_issue_url: +milestone: '14.9' +type: development +group: group::workspace +default_enabled: false diff --git a/config/feature_flags/ops/deprecated_serverless.yml b/config/feature_flags/ops/deprecated_serverless.yml new file mode 100644 index 00000000000..0982778f139 --- /dev/null +++ b/config/feature_flags/ops/deprecated_serverless.yml @@ -0,0 +1,8 @@ +--- +name: deprecated_serverless +introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/81493' +rollout_issue_url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/353901' +milestone: '14.9' +type: ops +group: 'group::configure' +default_enabled: true diff --git a/doc/administration/operations/puma.md b/doc/administration/operations/puma.md index c7df8249ae4..9e828b39f46 100644 --- a/doc/administration/operations/puma.md +++ b/doc/administration/operations/puma.md @@ -4,45 +4,19 @@ group: Distribution info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Puma **(FREE SELF)** +# Configure the bundled Puma instance of the GitLab package **(FREE SELF)** -Puma is a simple, fast, multi-threaded, and highly concurrent HTTP 1.1 server for -Ruby applications. It's the default GitLab web server since GitLab 13.0 -and has replaced Unicorn. From GitLab 14.0, Unicorn is no longer supported. +Puma is a fast, multi-threaded, and highly concurrent HTTP 1.1 server for +Ruby applications. It runs the core Rails application that provides the user-facing +features of GitLab. -NOTE: -Starting with GitLab 13.0, Puma is the default web server and Unicorn has been disabled. -In GitLab 14.0, Unicorn was removed from the Linux package and only Puma is available. +## Reducing memory use -## Configure Puma +To reduce memory use, Puma forks worker processes. Each time a worker is created, +it shares memory with the primary process. The worker uses additional memory only +when it changes or adds to its memory pages. -To configure Puma: - -1. Determine suitable Puma worker and thread [settings](../../install/requirements.md#puma-settings). -1. If you're switching from Unicorn, [convert any custom settings to Puma](#convert-unicorn-settings-to-puma). -1. For multi-node deployments, configure the load balancer to use the - [readiness check](../load_balancer.md#readiness-check). -1. Reconfigure GitLab so the above changes take effect: - - ```shell - sudo gitlab-ctl reconfigure - ``` - -For Helm-based deployments, see the -[`webservice` chart documentation](https://docs.gitlab.com/charts/charts/gitlab/webservice/index.html). - -For more details about the Puma configuration, see the -[Puma documentation](https://github.com/puma/puma#configuration). - -## Puma Worker Killer - -Puma forks worker processes as part of a strategy to reduce memory use. - -Each time a worker is created, it shares memory with the primary process and -only uses additional memory when it makes changes or additions to its memory pages. - -Memory use by workers therefore increases over time, and Puma Worker Killer is the -mechanism that recovers this memory. +Memory use increases over time, but you can use Puma Worker Killer to recover memory. By default: @@ -50,6 +24,8 @@ By default: exceeds a [memory limit](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/cluster/puma_worker_killer_initializer.rb). - Rolling restarts of Puma workers are performed every 12 hours. +### Change the memory limit setting + To change the memory limit setting: 1. Edit `/etc/gitlab/gitlab.rb`: @@ -58,26 +34,28 @@ To change the memory limit setting: puma['per_worker_max_memory_mb'] = 1024 ``` -1. Reconfigure GitLab for the changes to take effect: +1. Reconfigure GitLab: ```shell sudo gitlab-ctl reconfigure ``` -There are costs associated with killing and replacing workers including -reduced capacity to run GitLab, and CPU that is consumed -restarting the workers. `per_worker_max_memory_mb` should be set to a -higher value if the worker killer is replacing workers too often. +When workers are killed and replaced, capacity to run GitLab is reduced, +and CPU is consumed. Set `per_worker_max_memory_mb` to a higher value if the worker killer +is replacing workers too often. -Worker count is calculated based on CPU cores, so a small GitLab deployment +Worker count is calculated based on CPU cores. A small GitLab deployment with 4-8 workers may experience performance issues if workers are being restarted -frequently, once or more per minute. This is too often. +too often (once or more per minute). A higher value of `1200` or more would be beneficial if the server has free memory. -The worker killer checks every 20 seconds, and can be monitored using -[the Puma log](../logs.md#puma_stdoutlog) `/var/log/gitlab/puma/puma_stdout.log`. -For example, for GitLab 13.5: +### Monitor worker memory + +The worker killer checks memory every 20 seconds. + +To monitor the worker killer, use [the Puma log](../logs.md#puma_stdoutlog) `/var/log/gitlab/puma/puma_stdout.log`. +For example: ```plaintext PumaWorkerKiller: Out of memory. 4 workers consuming total: 4871.23828125 MB @@ -88,9 +66,9 @@ From this output: - The formula that calculates the maximum memory value results in workers being killed before they reach the `per_worker_max_memory_mb` value. -- The default values for the formula before GitLab 13.5 were 550MB for the primary - and `per_worker_max_memory_mb` specified 850MB for each worker. -- As of GitLab 13.5 the values are primary: 800MB, worker: 1024MB. +- In GitLab 13.4 and earlier, the default values for the formula were 550MB for the primary + and 850MB for each worker. +- In GitLab 13.5 and later, the values are primary: 800MB, worker: 1024MB. - The threshold for workers to be killed is set at 98% of the limit: ```plaintext @@ -102,16 +80,15 @@ From this output: Increasing the maximum to `1200`, for example, would set a `max: 5488 MB` value. -Workers use additional memory on top of the shared memory, how much +Workers use additional memory on top of the shared memory. The amount of memory depends on a site's use of GitLab. -## Worker timeout +## Change the worker timeout -A [timeout of 60 seconds](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/initializers/rack_timeout.rb) -is used when Puma is enabled. +The default Puma [timeout is 60 seconds](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/initializers/rack_timeout.rb). NOTE: -Unlike Unicorn, the `puma['worker_timeout']` setting does not set the maximum request duration. +The `puma['worker_timeout']` setting does not set the maximum request duration. To change the worker timeout to 600 seconds: @@ -123,26 +100,38 @@ To change the worker timeout to 600 seconds: } ``` -1. Reconfigure GitLab for the changes to take effect: +1. Reconfigure GitLab: ```shell sudo gitlab-ctl reconfigure ``` -## Memory-constrained environments +## Disable Puma clustered mode in memory-constrained environments In a memory-constrained environment with less than 4GB of RAM available, consider disabling Puma -[Clustered mode](https://github.com/puma/puma#clustered-mode). +[clustered mode](https://github.com/puma/puma#clustered-mode). -Configuring Puma by setting the amount of `workers` to `0` could reduce memory usage by hundreds of MB. +Set the number of `workers` to `0` to reduce memory usage by hundreds of MB: + +1. Edit `/etc/gitlab/gitlab.rb`: + + ```ruby + puma['worker_processes'] = 0 + ``` + +1. Reconfigure GitLab: + + ```shell + sudo gitlab-ctl reconfigure + ``` + +Unlike in a clustered mode, which is set up by default, only a single Puma process would serve the application. For details on Puma worker and thread settings, see the [Puma requirements](../../install/requirements.md#puma-settings). -Unlike in a Clustered mode, which is set up by default, only a single Puma process would serve the application. +The downside of running Puma in this configuration is the reduced throughput, which can be +considered a fair tradeoff in a memory-constrained environment. -The downside of running Puma with such configuration is the reduced throughput, which could be -considered as a fair tradeoff in a memory-constraint environment. - -When running Puma in Single mode, some features are not supported: +When running Puma in single mode, some features are not supported: - [Phased restart](https://gitlab.com/gitlab-org/gitlab/-/issues/300665) - [Puma Worker Killer](https://gitlab.com/gitlab-org/gitlab/-/issues/300664) @@ -151,22 +140,23 @@ To learn more, visit [epic 5303](https://gitlab.com/groups/gitlab-org/-/epics/53 ## Performance caveat when using Puma with Rugged -For deployments where NFS is used to store Git repository, we allow GitLab to use -[direct Git access](../gitaly/index.md#direct-access-to-git-in-gitlab) to improve performance using +For deployments where NFS is used to store Git repositories, GitLab uses +[direct Git access](../gitaly/index.md#direct-access-to-git-in-gitlab) to improve performance by using [Rugged](https://github.com/libgit2/rugged). Rugged usage is automatically enabled if direct Git access [is available](../gitaly/index.md#how-it-works) -and Puma is running single threaded, unless it is disabled by -[feature flags](../../development/gitaly.md#legacy-rugged-code). +and Puma is running single threaded, unless it is disabled by a +[feature flag](../../development/gitaly.md#legacy-rugged-code). -MRI Ruby uses a GVL. This allows MRI Ruby to be multi-threaded, but running at -most on a single core. Since Rugged can use a thread for long periods of -time (due to intensive I/O operations of Git access), this can starve other threads -that might be processing requests. This is not a case for Unicorn or Puma running -in a single thread mode, as concurrently at most one request is being processed. +MRI Ruby uses a Global VM Lock (GVL). GVL allows MRI Ruby to be multi-threaded, but running at +most on a single core. -We are actively working on removing Rugged usage. Even though performance without Rugged +Git includes intensive I/O operations. When Rugged uses a thread for a long period of time, +other threads that might be processing requests can starve. Puma running in single thread mode +does not have this issue, because concurrently at most one request is being processed. + +GitLab is working to remove Rugged usage. Even though performance without Rugged is acceptable today, in some cases it might be still beneficial to run with it. Given the caveat of running Rugged with multi-threaded Puma, and acceptable @@ -177,55 +167,70 @@ This default behavior may not be the optimal configuration in some situations. I plays an important role in your deployment, we suggest you benchmark to find the optimal configuration: -- The safest option is to start with single-threaded Puma. When working with - Rugged, single-threaded Puma works the same as Unicorn. -- To force Rugged to be used with multi-threaded Puma, you can use - [feature flags](../../development/gitaly.md#legacy-rugged-code). +- The safest option is to start with single-threaded Puma. +- To force Rugged to be used with multi-threaded Puma, you can use a + [feature flag](../../development/gitaly.md#legacy-rugged-code). -## Convert Unicorn settings to Puma +## Switch from Unicorn to Puma NOTE: -Starting with GitLab 13.0, Puma is the default web server and Unicorn has been -disabled by default. In GitLab 14.0, Unicorn was removed from the Linux package -and only Puma is available. +For Helm-based deployments, see the +[`webservice` chart documentation](https://docs.gitlab.com/charts/charts/gitlab/webservice/index.html). -Puma has a multi-thread architecture which uses less memory than a multi-process +Starting with GitLab 13.0, Puma is the default web server and Unicorn has been disabled. +In GitLab 14.0, [Unicorn was removed](../../update/removals.md#unicorn-in-gitlab-self-managed) +from the Linux package and is no longer supported. + +Puma has a multi-thread architecture that uses less memory than a multi-process application server like Unicorn. On GitLab.com, we saw a 40% reduction in memory -consumption. Most Rails applications requests normally include a proportion of I/O wait time. +consumption. Most Rails application requests normally include a proportion of I/O wait time. -During I/O wait time MRI Ruby releases the GVL (Global VM Lock) to other threads. +During I/O wait time, MRI Ruby releases the GVL to other threads. Multi-threaded Puma can therefore still serve more requests than a single process. When switching to Puma, any Unicorn server configuration will _not_ carry over automatically, due to differences between the two application servers. -The table below summarizes which Unicorn configuration keys correspond to those -in Puma when using the Linux package, and which ones have no corresponding counterpart. +To switch from Unicorn to Puma: -| Unicorn | Puma | -| ------------------------------------ | ---------------------------------- | -| `unicorn['enable']` | `puma['enable']` | -| `unicorn['worker_timeout']` | `puma['worker_timeout']` | -| `unicorn['worker_processes']` | `puma['worker_processes']` | -| n/a | `puma['ha']` | -| n/a | `puma['min_threads']` | -| n/a | `puma['max_threads']` | -| `unicorn['listen']` | `puma['listen']` | -| `unicorn['port']` | `puma['port']` | -| `unicorn['socket']` | `puma['socket']` | -| `unicorn['pidfile']` | `puma['pidfile']` | -| `unicorn['tcp_nopush']` | n/a | -| `unicorn['backlog_socket']` | n/a | -| `unicorn['somaxconn']` | `puma['somaxconn']` | -| n/a | `puma['state_path']` | -| `unicorn['log_directory']` | `puma['log_directory']` | -| `unicorn['worker_memory_limit_min']` | n/a | -| `unicorn['worker_memory_limit_max']` | `puma['per_worker_max_memory_mb']` | -| `unicorn['exporter_enabled']` | `puma['exporter_enabled']` | -| `unicorn['exporter_address']` | `puma['exporter_address']` | -| `unicorn['exporter_port']` | `puma['exporter_port']` | +1. Determine suitable Puma [worker and thread settings](../../install/requirements.md#puma-settings). +1. Convert any custom Unicorn settings to Puma. -## Puma exporter + The table below summarizes which Unicorn configuration keys correspond to those + in Puma when using the Linux package, and which ones have no corresponding counterpart. -You can use the Puma exporter to measure various Puma metrics. For more information, see -[Puma exporter](../monitoring/prometheus/puma_exporter.md). + | Unicorn | Puma | + | ------------------------------------ | ---------------------------------- | + | `unicorn['enable']` | `puma['enable']` | + | `unicorn['worker_timeout']` | `puma['worker_timeout']` | + | `unicorn['worker_processes']` | `puma['worker_processes']` | + | n/a | `puma['ha']` | + | n/a | `puma['min_threads']` | + | n/a | `puma['max_threads']` | + | `unicorn['listen']` | `puma['listen']` | + | `unicorn['port']` | `puma['port']` | + | `unicorn['socket']` | `puma['socket']` | + | `unicorn['pidfile']` | `puma['pidfile']` | + | `unicorn['tcp_nopush']` | n/a | + | `unicorn['backlog_socket']` | n/a | + | `unicorn['somaxconn']` | `puma['somaxconn']` | + | n/a | `puma['state_path']` | + | `unicorn['log_directory']` | `puma['log_directory']` | + | `unicorn['worker_memory_limit_min']` | n/a | + | `unicorn['worker_memory_limit_max']` | `puma['per_worker_max_memory_mb']` | + | `unicorn['exporter_enabled']` | `puma['exporter_enabled']` | + | `unicorn['exporter_address']` | `puma['exporter_address']` | + | `unicorn['exporter_port']` | `puma['exporter_port']` | + +1. Reconfigure GitLab: + + ```shell + sudo gitlab-ctl reconfigure + ``` + +1. Optional. For multi-node deployments, configure the load balancer to use the + [readiness check](../load_balancer.md#readiness-check). + +## Related topics + +- [Use the Puma exporter to measure various Puma metrics](../monitoring/prometheus/puma_exporter.md) diff --git a/doc/administration/troubleshooting/debug.md b/doc/administration/troubleshooting/debug.md index 744e12d4da1..0520ce470cb 100644 --- a/doc/administration/troubleshooting/debug.md +++ b/doc/administration/troubleshooting/debug.md @@ -225,7 +225,7 @@ gitlab_rails['env'] = { ``` For source installations, set the environment variable. -Refer to [Puma Worker timeout](../operations/puma.md#worker-timeout). +Refer to [Puma Worker timeout](../operations/puma.md#change-the-worker-timeout). [Reconfigure](../restart_gitlab.md#omnibus-gitlab-reconfigure) GitLab for the changes to take effect. diff --git a/doc/development/permissions.md b/doc/development/permissions.md index f994fcae228..47aebc2f4d2 100644 --- a/doc/development/permissions.md +++ b/doc/development/permissions.md @@ -9,6 +9,23 @@ info: To determine the technical writer assigned to the Stage/Group associated w There are multiple types of permissions across GitLab, and when implementing anything that deals with permissions, all of them should be considered. +## Instance + +### User types + +Each user can be one of the following types: + +- Regular. +- External - access to groups and projects only if direct member. +- [Internal users](internal_users.md) - system created. +- [Auditor](https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/app/policies/ee/base_policy.rb#L9): + - No access to projects or groups settings menu. + - No access to Admin Area. + - Read-only access to everything else. +- [Administrator](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/policies/base_policy.rb#L6) - read-write access. + +See the [permissions page](../user/permissions.md) for details on how each user type is used. + ## Groups and Projects ### General permissions diff --git a/doc/install/requirements.md b/doc/install/requirements.md index cacf23bbe21..11f623641c1 100644 --- a/doc/install/requirements.md +++ b/doc/install/requirements.md @@ -258,7 +258,7 @@ works. ### Puma per worker maximum memory By default, each Puma worker will be limited to 1024 MB of memory. -This setting [can be adjusted](../administration/operations/puma.md#puma-worker-killer) and should be considered +This setting [can be adjusted](../administration/operations/puma.md#change-the-memory-limit-setting) and should be considered if you need to increase the number of Puma workers. ## Redis and Sidekiq diff --git a/doc/user/admin_area/settings/gitaly_timeouts.md b/doc/user/admin_area/settings/gitaly_timeouts.md index fac23cb48af..42e0c9faf9f 100644 --- a/doc/user/admin_area/settings/gitaly_timeouts.md +++ b/doc/user/admin_area/settings/gitaly_timeouts.md @@ -22,6 +22,6 @@ The following timeouts are available. | Timeout | Default | Description | |:--------|:-----------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| Default | 55 seconds | Timeout for most Gitaly calls (not enforced for `git` `fetch` and `push` operations, or Sidekiq jobs). For example, checking if a repository exists on disk. Makes sure that Gitaly calls made within a web request cannot exceed the entire request timeout. It should be shorter than the [worker timeout](../../../administration/operations/puma.md#worker-timeout) that can be configured for [Puma](../../../install/requirements.md#puma-settings). If a Gitaly call timeout exceeds the worker timeout, the remaining time from the worker timeout is used to avoid having to terminate the worker. | +| Default | 55 seconds | Timeout for most Gitaly calls (not enforced for `git` `fetch` and `push` operations, or Sidekiq jobs). For example, checking if a repository exists on disk. Makes sure that Gitaly calls made within a web request cannot exceed the entire request timeout. It should be shorter than the [worker timeout](../../../administration/operations/puma.md#change-the-worker-timeout) that can be configured for [Puma](../../../install/requirements.md#puma-settings). If a Gitaly call timeout exceeds the worker timeout, the remaining time from the worker timeout is used to avoid having to terminate the worker. | | Fast | 10 seconds | Timeout for fast Gitaly operations used within requests, sometimes multiple times. For example, checking if a repository exists on disk. If fast operations exceed this threshold, there may be a problem with a storage shard. Failing fast can help maintain the stability of the GitLab instance. | | Medium | 30 seconds | Timeout for Gitaly operations that should be fast (possibly within requests) but preferably not used multiple times within a request. For example, loading blobs. Timeout that should be set between Default and Fast. | diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index 9fddb80d8bf..78c96fb9e9e 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -299,7 +299,7 @@ for `shared_buffers` is quite high, and we are ## Puma -GitLab.com uses the default of 60 seconds for [Puma request timeouts](../../administration/operations/puma.md#worker-timeout). +GitLab.com uses the default of 60 seconds for [Puma request timeouts](../../administration/operations/puma.md#change-the-worker-timeout). ## GitLab.com-specific rate limits diff --git a/doc/user/group/index.md b/doc/user/group/index.md index 54d9b2b5405..c2a42fcd072 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -806,9 +806,7 @@ The group's new subgroups have push rules set for them based on either: > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/285458) in GitLab 13.9. [Deployed behind the `group_merge_request_approval_settings_feature_flag` flag](../../administration/feature_flags.md), disabled by default. > - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/285410) in GitLab 14.5. - -FLAG: -On self-managed GitLab, by default this feature is available. To hide the feature per group, ask an administrator to [disable the feature flag](../../administration/feature_flags.md) named `group_merge_request_approval_settings_feature_flag`. On GitLab.com, this feature is available. +> - [Feature flag `group_merge_request_approval_settings_feature_flag`](https://gitlab.com/gitlab-org/gitlab/-/issues/343872) removed in GitLab 14.9. Group approval rules manage [project merge request approval rules](../project/merge_requests/approvals/index.md) at the top-level group level. These rules [cascade to all projects](../project/merge_requests/approvals/settings.md#settings-cascading) diff --git a/doc/user/index.md b/doc/user/index.md index 86dcfd8bb5c..4ce46c5b46f 100644 --- a/doc/user/index.md +++ b/doc/user/index.md @@ -76,12 +76,15 @@ You can also [integrate](project/integrations/overview.md) GitLab with numerous There are several types of users in GitLab: -- Regular users and GitLab.com users. -- [Groups](group/index.md) of users. -- GitLab [administrator area](admin_area/index.md) user. +- Regular users. +- [Internal users](../development/internal_users.md) often referred to as bot or system users. +- [Auditor](permissions.md#auditor-users) with read access to self-managed instances. - [GitLab Administrator](../administration/index.md) with full access to - self-managed instances' features and settings. -- [Internal users](../development/internal_users.md). + self-managed instances including settings and the [Admin Area](admin_area/index.md). + +Each user can be a member in a [group](group/index.md). + +See the [permissions page](permissions.md) for details on how each user type is used. ## User activity diff --git a/doc/user/project/merge_requests/approvals/settings.md b/doc/user/project/merge_requests/approvals/settings.md index 1e1c8ccb241..0a7fbc9ee95 100644 --- a/doc/user/project/merge_requests/approvals/settings.md +++ b/doc/user/project/merge_requests/approvals/settings.md @@ -140,9 +140,7 @@ To learn more, see [Coverage check approval rule](../../../../ci/pipelines/setti > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/285410) in GitLab 14.4. [Deployed behind the `group_merge_request_approval_settings_feature_flag` flag](../../../../administration/feature_flags.md), disabled by default. > - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/285410) in GitLab 14.5. - -FLAG: -On self-managed GitLab, by default this feature is available. To hide the feature per group, ask an administrator to [disable the feature flag](../../../../administration/feature_flags.md) named `group_merge_request_approval_settings_feature_flag`. On GitLab.com, this feature is available. +> - [Feature flag `group_merge_request_approval_settings_feature_flag`](https://gitlab.com/gitlab-org/gitlab/-/issues/343872) removed in GitLab 14.9. You can also enforce merge request approval settings: diff --git a/lib/serializers/unsafe_json.rb b/lib/serializers/unsafe_json.rb new file mode 100644 index 00000000000..25ec52e4581 --- /dev/null +++ b/lib/serializers/unsafe_json.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Serializers + class UnsafeJson + class << self + def dump(obj) + obj.to_json(unsafe: true) + end + + delegate :load, to: :JSON + end + end +end diff --git a/lib/sidebars/projects/menus/infrastructure_menu.rb b/lib/sidebars/projects/menus/infrastructure_menu.rb index 51580e91976..c012b3bb627 100644 --- a/lib/sidebars/projects/menus/infrastructure_menu.rb +++ b/lib/sidebars/projects/menus/infrastructure_menu.rb @@ -64,7 +64,7 @@ module Sidebars end def serverless_menu_item - unless can?(context.current_user, :read_cluster, context.project) + unless Feature.enabled?(:deprecated_serverless, context.project, default_enabled: :yaml, type: :ops) && can?(context.current_user, :read_cluster, context.project) return ::Sidebars::NilMenuItem.new(item_id: :serverless) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7c7e919fa88..a4d612ca20c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7453,6 +7453,15 @@ msgstr "" msgid "Clear templates search input" msgstr "" +msgid "Clear this checkbox to use a personal access token instead." +msgstr "" + +msgid "Clear this checkbox to use a personal access token or LDAP password instead." +msgstr "" + +msgid "Clear this checkbox to use an external authentication provider instead." +msgstr "" + msgid "Clear weight" msgstr "" @@ -41344,9 +41353,6 @@ msgstr "" msgid "When enabled, job logs are collected by Datadog and displayed along with pipeline execution traces." msgstr "" -msgid "When inactive, an external authentication provider must be used." -msgstr "" - msgid "When merge requests and commits in the default branch close, any issues they reference also close." msgstr "" diff --git a/qa/qa/page/project/web_ide/edit.rb b/qa/qa/page/project/web_ide/edit.rb index 403c919c6e5..435cc4a717e 100644 --- a/qa/qa/page/project/web_ide/edit.rb +++ b/qa/qa/page/project/web_ide/edit.rb @@ -32,7 +32,7 @@ module QA element :file_template_dropdown end - view 'app/assets/javascripts/ide/components/file_templates/dropdown.vue' do + view 'app/assets/javascripts/ide/components/file_templates/bar.vue' do element :dropdown_filter_input end diff --git a/qa/qa/tools/delete_test_users.rb b/qa/qa/tools/delete_test_users.rb index 1f69f1bc548..c60e6a6af44 100644 --- a/qa/qa/tools/delete_test_users.rb +++ b/qa/qa/tools/delete_test_users.rb @@ -5,8 +5,8 @@ # - If `dry_run` is true the script will list the users to be deleted by username, but it won't delete them # - Specify `exclude_users` as a comma-separated list of usernames to not delete. # -# Required environment variables: GITLAB_QA_ACCESS_TOKEN and GITLAB_ADDRESS -# - GITLAB_QA_ACCESS_TOKEN must have admin API access +# Required environment variables: GITLAB_QA_ADMIN_ACCESS_TOKEN and GITLAB_ADDRESS +# - GITLAB_QA_ADMIN_ACCESS_TOKEN must have admin API access module QA module Tools @@ -19,9 +19,9 @@ module QA def initialize(delete_before: (Date.today - 1).to_s, dry_run: 'false', exclude_users: nil) raise ArgumentError, "Please provide GITLAB_ADDRESS" unless ENV['GITLAB_ADDRESS'] - raise ArgumentError, "Please provide GITLAB_QA_ACCESS_TOKEN" unless ENV['GITLAB_QA_ACCESS_TOKEN'] + raise ArgumentError, "Please provide GITLAB_QA_ADMIN_ACCESS_TOKEN" unless ENV['GITLAB_QA_ADMIN_ACCESS_TOKEN'] - @api_client = Runtime::API::Client.new(ENV['GITLAB_ADDRESS'], personal_access_token: ENV['GITLAB_QA_ACCESS_TOKEN']) + @api_client = Runtime::API::Client.new(ENV['GITLAB_ADDRESS'], personal_access_token: ENV['GITLAB_QA_ADMIN_ACCESS_TOKEN']) @dry_run = !FALSY_VALUES.include?(dry_run.to_s.downcase) @delete_before = Date.parse(delete_before) @page_no = '1' @@ -29,7 +29,7 @@ module QA end def run - puts "Deleting users with a username starting with 'qa-user-' created before #{@delete_before}..." + puts "Deleting users with a username starting with 'qa-user-' or 'test-user-' created before #{@delete_before}..." while page_no.present? users = fetch_test_users diff --git a/spec/controllers/boards/lists_controller_spec.rb b/spec/controllers/boards/lists_controller_spec.rb index 29141582c6f..95334974e66 100644 --- a/spec/controllers/boards/lists_controller_spec.rb +++ b/spec/controllers/boards/lists_controller_spec.rb @@ -208,7 +208,7 @@ RSpec.describe Boards::ListsController do sign_in(user) params = { namespace_id: project.namespace.to_param, - project_id: project, + project_id: project.id, board_id: board.to_param, id: list.to_param, list: { position: position }, @@ -221,7 +221,7 @@ RSpec.describe Boards::ListsController do sign_in(user) params = { namespace_id: project.namespace.to_param, - project_id: project, + project_id: project.id, board_id: board.to_param, id: list.to_param, list: setting, diff --git a/spec/controllers/projects/serverless/functions_controller_spec.rb b/spec/controllers/projects/serverless/functions_controller_spec.rb index 860bbc1c5cc..f8cee09006c 100644 --- a/spec/controllers/projects/serverless/functions_controller_spec.rb +++ b/spec/controllers/projects/serverless/functions_controller_spec.rb @@ -39,9 +39,24 @@ RSpec.describe Projects::Serverless::FunctionsController do project_id: project.to_param) end + shared_examples_for 'behind :deprecated_serverless feature flag' do + before do + stub_feature_flags(deprecated_serverless: false) + end + + it 'returns 404' do + action + expect(response).to have_gitlab_http_status(:not_found) + end + end + describe 'GET #index' do let(:expected_json) { { 'knative_installed' => knative_state, 'functions' => functions } } + it_behaves_like 'behind :deprecated_serverless feature flag' do + let(:action) { get :index, params: params({ format: :json }) } + end + context 'when cache is being read' do let(:knative_state) { 'checking' } let(:functions) { [] } @@ -147,6 +162,10 @@ RSpec.describe Projects::Serverless::FunctionsController do end describe 'GET #show' do + it_behaves_like 'behind :deprecated_serverless feature flag' do + let(:action) { get :show, params: params({ format: :json, environment_id: "*", id: "foo" }) } + end + context 'with function that does not exist' do it 'returns 404' do get :show, params: params({ format: :json, environment_id: "*", id: "foo" }) @@ -239,6 +258,10 @@ RSpec.describe Projects::Serverless::FunctionsController do end describe 'GET #metrics' do + it_behaves_like 'behind :deprecated_serverless feature flag' do + let(:action) { get :metrics, params: params({ format: :json, environment_id: "*", id: "foo" }) } + end + context 'invalid data' do it 'has a bad function name' do get :metrics, params: params({ format: :json, environment_id: "*", id: "foo" }) diff --git a/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb b/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb index 0840ccf19e4..ef23725c790 100644 --- a/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb +++ b/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Banzai::Filter::IssuableReferenceExpansionFilter do end it 'ignores non-issuable links' do - link = create_link('text', project: project, reference_type: 'issue') + link = create_link('text', project: project.id, reference_type: 'issue') doc = filter(link, context) expect(doc.css('a').last.text).to eq('text') diff --git a/spec/lib/serializers/unsafe_json_spec.rb b/spec/lib/serializers/unsafe_json_spec.rb new file mode 100644 index 00000000000..9bf04f8f4aa --- /dev/null +++ b/spec/lib/serializers/unsafe_json_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'oj' + +RSpec.describe Serializers::UnsafeJson do + let(:result) { double(:result) } + + describe '.dump' do + let(:obj) { { key: "value" } } + + it 'calls object#to_json with unsafe: true and returns the result' do + expect(obj).to receive(:to_json).with(unsafe: true).and_return(result) + expect(described_class.dump(obj)).to eq(result) + end + end + + describe '.load' do + let(:data_string) { '{"key":"value","variables":[{"key":"VAR1","value":"VALUE1"}]}' } + let(:data_hash) { Gitlab::Json.parse(data_string) } + + it 'calls JSON.load and returns the result' do + expect(JSON).to receive(:load).with(data_hash).and_return(result) + expect(described_class.load(data_hash)).to eq(result) + end + end +end diff --git a/spec/lib/sidebars/projects/menus/infrastructure_menu_spec.rb b/spec/lib/sidebars/projects/menus/infrastructure_menu_spec.rb index 0e415ec6014..8a6b0e4e95d 100644 --- a/spec/lib/sidebars/projects/menus/infrastructure_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/infrastructure_menu_spec.rb @@ -92,6 +92,14 @@ RSpec.describe Sidebars::Projects::Menus::InfrastructureMenu do let(:item_id) { :serverless } it_behaves_like 'access rights checks' + + context 'when feature :deprecated_serverless is disabled' do + before do + stub_feature_flags(deprecated_serverless: false) + end + + it { is_expected.to be_nil } + end end describe 'Terraform' do diff --git a/spec/models/concerns/blocks_json_serialization_spec.rb b/spec/models/concerns/blocks_json_serialization_spec.rb deleted file mode 100644 index d811b47fa35..00000000000 --- a/spec/models/concerns/blocks_json_serialization_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe BlocksJsonSerialization do - before do - stub_const('DummyModel', Class.new) - DummyModel.class_eval do - include BlocksJsonSerialization - end - end - - it 'blocks as_json' do - expect { DummyModel.new.as_json } - .to raise_error(described_class::JsonSerializationError, /DummyModel/) - end - - it 'blocks to_json' do - expect { DummyModel.new.to_json } - .to raise_error(described_class::JsonSerializationError, /DummyModel/) - end -end diff --git a/spec/models/concerns/blocks_unsafe_serialization_spec.rb b/spec/models/concerns/blocks_unsafe_serialization_spec.rb new file mode 100644 index 00000000000..5c8f5035a58 --- /dev/null +++ b/spec/models/concerns/blocks_unsafe_serialization_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BlocksUnsafeSerialization do + before do + stub_const('DummyModel', Class.new) + DummyModel.class_eval do + include ActiveModel::Serializers::JSON + include BlocksUnsafeSerialization + end + end + + it_behaves_like 'blocks unsafe serialization' do + let(:object) { DummyModel.new } + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 6516b5f777e..f65365db8a8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -8022,6 +8022,20 @@ RSpec.describe Project, factory_default: :keep do end end + describe 'serialization' do + let(:object) { build(:project) } + + it_behaves_like 'blocks unsafe serialization' + + context 'when feature flag block_project_serialization is disabled' do + before do + stub_feature_flags(block_project_serialization: false) + end + + it_behaves_like 'allows unsafe serialization' + end + end + describe '#runners_token' do let_it_be(:project) { create(:project) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a5bf32972a2..49f041947b4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -17,7 +17,7 @@ RSpec.describe User do it { is_expected.to include_module(Referable) } it { is_expected.to include_module(Sortable) } it { is_expected.to include_module(TokenAuthenticatable) } - it { is_expected.to include_module(BlocksJsonSerialization) } + it { is_expected.to include_module(BlocksUnsafeSerialization) } it { is_expected.to include_module(AsyncDeviseEmail) } end diff --git a/spec/support/shared_examples/blocks_unsafe_serialization_shared_examples.rb b/spec/support/shared_examples/blocks_unsafe_serialization_shared_examples.rb new file mode 100644 index 00000000000..db42e41344f --- /dev/null +++ b/spec/support/shared_examples/blocks_unsafe_serialization_shared_examples.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Requires a context with: +# - object +# +RSpec.shared_examples 'blocks unsafe serialization' do + it 'blocks as_json' do + expect { object.as_json }.to raise_error(described_class::UnsafeSerializationError, /#{object.class.name}/) + end + + it 'blocks to_json' do + expect { object.to_json }.to raise_error(described_class::UnsafeSerializationError, /#{object.class.name}/) + end +end + +RSpec.shared_examples 'allows unsafe serialization' do + it 'allows as_json' do + expect { object.as_json }.not_to raise_error + end + + it 'allows to_json' do + expect { object.to_json }.not_to raise_error + end +end diff --git a/spec/support/shared_examples/serializers/note_entity_shared_examples.rb b/spec/support/shared_examples/serializers/note_entity_shared_examples.rb index 9af6ec45e49..2e557ca090c 100644 --- a/spec/support/shared_examples/serializers/note_entity_shared_examples.rb +++ b/spec/support/shared_examples/serializers/note_entity_shared_examples.rb @@ -68,6 +68,29 @@ RSpec.shared_examples 'note entity' do end end + describe ':outdated_line_change_path' do + before do + allow(note).to receive(:show_outdated_changes?).and_return(show_outdated_changes) + end + + context 'when note shows outdated changes' do + let(:show_outdated_changes) { true } + + it 'returns correct outdated_line_change_namespace_project_note_path' do + path = "/#{note.project.namespace.path}/#{note.project.path}/notes/#{note.id}/outdated_line_change" + expect(subject[:outdated_line_change_path]).to eq(path) + end + end + + context 'when note does not show outdated changes' do + let(:show_outdated_changes) { false } + + it 'does not expose outdated_line_change_path' do + expect(subject).not_to include(:outdated_line_change_path) + end + end + end + context 'when note was edited' do before do note.update!(updated_at: 1.minute.from_now, updated_by: user)