Add latest changes from gitlab-org/gitlab@master

This commit is contained in:
GitLab Bot 2021-10-07 15:12:00 +00:00
parent 43c3400c67
commit c3524d16b2
45 changed files with 1781 additions and 120 deletions

View File

@ -1,6 +1,8 @@
# frozen_string_literal: true # frozen_string_literal: true
class GraphqlController < ApplicationController class GraphqlController < ApplicationController
extend ::Gitlab::Utils::Override
# Unauthenticated users have access to the API for public data # Unauthenticated users have access to the API for public data
skip_before_action :authenticate_user! skip_before_action :authenticate_user!
@ -35,6 +37,7 @@ class GraphqlController < ApplicationController
# callback execution order here # callback execution order here
around_action :sessionless_bypass_admin_mode!, if: :sessionless_user? around_action :sessionless_bypass_admin_mode!, if: :sessionless_user?
# The default feature category is overridden to read from request
feature_category :not_owned feature_category :not_owned
def execute def execute
@ -64,6 +67,11 @@ class GraphqlController < ApplicationController
render_error(exception.message, status: :unprocessable_entity) render_error(exception.message, status: :unprocessable_entity)
end end
override :feature_category
def feature_category
::Gitlab::FeatureCategories.default.from_request(request) || super
end
private private
def disallow_mutations_for_get def disallow_mutations_for_get

View File

@ -18,11 +18,8 @@ module Resolvers
filter_params = filters.merge(board_id: list.board.id, id: list.id) filter_params = filters.merge(board_id: list.board.id, id: list.id)
service = ::Boards::Issues::ListService.new(list.board.resource_parent, context[:current_user], filter_params) service = ::Boards::Issues::ListService.new(list.board.resource_parent, context[:current_user], filter_params)
pagination_connections = Gitlab::Graphql::Pagination::Keyset::Connection.new(service.execute)
::Boards::Issues::ListService.initialize_relative_positions(list.board, current_user, pagination_connections.items) service.execute
pagination_connections
end end
# https://gitlab.com/gitlab-org/gitlab/-/issues/235681 # https://gitlab.com/gitlab-org/gitlab/-/issues/235681

View File

@ -21,6 +21,7 @@ module Types
@feature_flag = kwargs[:feature_flag] @feature_flag = kwargs[:feature_flag]
kwargs = check_feature_flag(kwargs) kwargs = check_feature_flag(kwargs)
@deprecation = gitlab_deprecation(kwargs) @deprecation = gitlab_deprecation(kwargs)
after_connection_extensions = kwargs.delete(:late_extensions) || []
super(**kwargs, &block) super(**kwargs, &block)
@ -28,6 +29,8 @@ module Types
extension ::Gitlab::Graphql::CallsGitaly::FieldExtension if Gitlab.dev_or_test_env? extension ::Gitlab::Graphql::CallsGitaly::FieldExtension if Gitlab.dev_or_test_env?
extension ::Gitlab::Graphql::Present::FieldExtension extension ::Gitlab::Graphql::Present::FieldExtension
extension ::Gitlab::Graphql::Authorize::ConnectionFilterExtension extension ::Gitlab::Graphql::Authorize::ConnectionFilterExtension
after_connection_extensions.each { extension _1 } if after_connection_extensions.any?
end end
def may_call_gitaly? def may_call_gitaly?

View File

@ -27,6 +27,7 @@ module Types
field :issues, ::Types::IssueType.connection_type, null: true, field :issues, ::Types::IssueType.connection_type, null: true,
description: 'Board issues.', description: 'Board issues.',
late_extensions: [Gitlab::Graphql::Board::IssuesConnectionExtension],
resolver: ::Resolvers::BoardListIssuesResolver resolver: ::Resolvers::BoardListIssuesResolver
def issues_count def issues_count

View File

@ -3,7 +3,13 @@
class UserHighestRole < ApplicationRecord class UserHighestRole < ApplicationRecord
belongs_to :user, optional: false belongs_to :user, optional: false
validates :highest_access_level, allow_nil: true, inclusion: { in: Gitlab::Access.all_values } validates :highest_access_level, allow_nil: true, inclusion: { in: ->(_) { self.allowed_values } }
scope :with_highest_access_level, -> (highest_access_level) { where(highest_access_level: highest_access_level) } scope :with_highest_access_level, -> (highest_access_level) { where(highest_access_level: highest_access_level) }
def self.allowed_values
Gitlab::Access.all_values
end
end end
UserHighestRole.prepend_mod

View File

@ -103,42 +103,40 @@ module Ci
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def each_build(params, &blk) def each_build(params, &blk)
::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/339429') do queue = ::Ci::Queue::BuildQueueService.new(runner)
queue = ::Ci::Queue::BuildQueueService.new(runner)
builds = begin builds = begin
if runner.instance_type? if runner.instance_type?
queue.builds_for_shared_runner queue.builds_for_shared_runner
elsif runner.group_type? elsif runner.group_type?
queue.builds_for_group_runner queue.builds_for_group_runner
else else
queue.builds_for_project_runner queue.builds_for_project_runner
end
end end
if runner.ref_protected?
builds = queue.builds_for_protected_runner(builds)
end
# pick builds that does not have other tags than runner's one
builds = queue.builds_matching_tag_ids(builds, runner.tags.ids)
# pick builds that have at least one tag
unless runner.run_untagged?
builds = queue.builds_with_any_tags(builds)
end
# pick builds that older than specified age
if params.key?(:job_age)
builds = queue.builds_queued_before(builds, params[:job_age].seconds.ago)
end
build_ids = retrieve_queue(-> { queue.execute(builds) })
@metrics.observe_queue_size(-> { build_ids.size }, @runner.runner_type)
build_ids.each { |build_id| yield Ci::Build.find(build_id) }
end end
if runner.ref_protected?
builds = queue.builds_for_protected_runner(builds)
end
# pick builds that does not have other tags than runner's one
builds = queue.builds_matching_tag_ids(builds, runner.tags.ids)
# pick builds that have at least one tag
unless runner.run_untagged?
builds = queue.builds_with_any_tags(builds)
end
# pick builds that older than specified age
if params.key?(:job_age)
builds = queue.builds_queued_before(builds, params[:job_age].seconds.ago)
end
build_ids = retrieve_queue(-> { queue.execute(builds) })
@metrics.observe_queue_size(-> { build_ids.size }, @runner.runner_type)
build_ids.each { |build_id| yield Ci::Build.find(build_id) }
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord

View File

@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/297382
milestone: '13.11' milestone: '13.11'
type: development type: development
group: group::runner group: group::runner
default_enabled: false default_enabled: true

View File

@ -2,8 +2,7 @@
Gitlab::Database::Partitioning.register_models([ Gitlab::Database::Partitioning.register_models([
AuditEvent, AuditEvent,
WebHookLog, WebHookLog
LooseForeignKeys::DeletedRecord
]) ])
if Gitlab.ee? if Gitlab.ee?

View File

@ -65,9 +65,10 @@ because the expansion is done in GitLab before any runner gets the job.
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48627) in GitLab 13.10. [Deployed behind the `variable_inside_variable` feature flag](../../user/feature_flags.md), disabled by default. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48627) in GitLab 13.10. [Deployed behind the `variable_inside_variable` feature flag](../../user/feature_flags.md), disabled by default.
> - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/297382) in GitLab 14.3. > - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/297382) in GitLab 14.3.
> - [Enabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/297382) in GitLab 14.4.
FLAG: FLAG:
On self-managed GitLab, by default this feature is disabled. To enable the feature per project or for your entire instance, ask an administrator to [enable the `variable_inside_variable` flag](../../administration/feature_flags.md). On self-managed GitLab, by default this feature is available. To hide the feature, ask an administrator to [disable the feature flag](../../administration/feature_flags.md) named `variable_inside_variable`. On GitLab.com, this feature is available.
GitLab expands job variable values recursively before sending them to the runner. For example: GitLab expands job variable values recursively before sending them to the runner. For example:

View File

@ -169,7 +169,7 @@ Consider the following scope:
scope = Issue.where(project_id: 10).order(Gitlab::Database.nulls_last_order('relative_position', 'DESC')) scope = Issue.where(project_id: 10).order(Gitlab::Database.nulls_last_order('relative_position', 'DESC'))
# SELECT "issues".* FROM "issues" WHERE "issues"."project_id" = 10 ORDER BY relative_position DESC NULLS LAST # SELECT "issues".* FROM "issues" WHERE "issues"."project_id" = 10 ORDER BY relative_position DESC NULLS LAST
scope.keyset_paginate # raises: Gitlab::Pagination::Keyset::Paginator::UnsupportedScopeOrder: The order on the scope does not support keyset pagination scope.keyset_paginate # raises: Gitlab::Pagination::Keyset::UnsupportedScopeOrder: The order on the scope does not support keyset pagination
``` ```
The `keyset_paginate` method raises an error because the order value on the query is a custom SQL string and not an [`Arel`](https://www.rubydoc.info/gems/arel) AST node. The keyset library cannot automatically infer configuration values from these kinds of queries. The `keyset_paginate` method raises an error because the order value on the query is a custom SQL string and not an [`Arel`](https://www.rubydoc.info/gems/arel) AST node. The keyset library cannot automatically infer configuration values from these kinds of queries.

View File

@ -425,6 +425,92 @@ to fix the cross-join. If the cross-join is being used in a migration, we do not
need to fix the code. See <https://gitlab.com/gitlab-org/gitlab/-/issues/340017> need to fix the code. See <https://gitlab.com/gitlab-org/gitlab/-/issues/340017>
for more details. for more details.
### Removing cross-database transactions
When dealing with multiple databases, it's important to pay close attention to data modification
that affects more than one database.
[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/339811) GitLab 14.4, an automated check
prevents cross-database modifications.
When at least two different databases are modified during a transaction initiated on any database
server, the application triggers a cross-database modification error (only in test environment).
Example:
```ruby
# Open transaction on Main DB
ApplicationRecord.transaction do
ci_build.update!(updated_at: Time.current) # UPDATE on CI DB
ci_build.project.update!(updated_at: Time.current) # UPDATE on Main DB
end
# raises error: Cross-database data modification of 'main, ci' were detected within
# a transaction modifying the 'ci_build, projects' tables
```
The code example above updates the timestamp for two records within a transaction. With the
ongoing work on the CI database decomposition, we cannot ensure the schematics of a database
transaction.
If the second update query fails, the first update query will not be
rolled back because the `ci_build` record is located on a different database server. For
more information, look at the
[transaction guidelines](transaction_guidelines.md#dangerous-example-third-party-api-calls)
page.
#### Fixing cross-database errors
##### Removing the transaction block
Without an open transaction, the cross-database modification check cannot raise an error.
By making this change, we sacrifice consistency. In case of an application failure after the
first `UPDATE` query, the second `UPDATE` query will never execute.
The same code without the `transaction` block:
```ruby
ci_build.update!(updated_at: Time.current) # CI DB
ci_build.project.update!(updated_at: Time.current) # Main DB
```
##### Async processing
If we need more guarantee that an operation finishes the work consistently we can execute it
within a background job. A background job is scheduled asynchronously and retried several times
in case of an error. There is still a very small chance of introducing inconsistency.
Example:
```ruby
current_time = Time.current
MyAsyncConsistencyJob.perform_async(cu_build.id)
ci_build.update!(updated_at: current_time)
ci_build.project.update!(updated_at: current_time)
```
The `MyAsyncConsistencyJob` would also attempt to update the timestamp if they differ.
##### Aiming for perfect consistency
At this point, we don't have the tooling (we might not even need it) to ensure similar consistency
characteristics as we had with one database. If you think that the code you're working on requires
these properties, then you can disable the cross-database modification check by wrapping to
offending database queries with a block and create a follow-up issue mentioning the sharding group
(`gitlab-org/sharding-group`).
```ruby
Gitlab::Database.allow_cross_joins_across_databases(url: 'gitlab issue URL') do
ApplicationRecord.transaction do
ci_build.update!(updated_at: Time.current) # UPDATE on CI DB
ci_build.project.update!(updated_at: Time.current) # UPDATE on Main DB
end
end
```
Don't hesitate to reach out to the
[sharding group](https://about.gitlab.com/handbook/engineering/development/enablement/sharding)
for advice.
## `config/database.yml` ## `config/database.yml`
GitLab will support running multiple databases in the future, for example to [separate tables for the continuous integration features](https://gitlab.com/groups/gitlab-org/-/epics/6167) from the main database. In order to prepare for this change, we [validate the structure of the configuration](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67877) in `database.yml` to ensure that only known databases are used. GitLab will support running multiple databases in the future, for example to [separate tables for the continuous integration features](https://gitlab.com/groups/gitlab-org/-/epics/6167) from the main database. In order to prepare for this change, we [validate the structure of the configuration](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67877) in `database.yml` to ensure that only known databases are used.

View File

@ -6,11 +6,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w
# Award emoji **(FREE)** # Award emoji **(FREE)**
> - [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/1825) in GitLab 8.2.
> - GitLab 9.0 [introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/9570) the usage of native emoji if the platform
> supports them and falls back to images or CSS sprites. This change greatly
> improved award emoji performance overall.
When you're collaborating online, you get fewer opportunities for high-fives When you're collaborating online, you get fewer opportunities for high-fives
and thumbs-ups. Emoji can be awarded to [issues](project/issues/index.md), [merge requests](project/merge_requests/index.md), and thumbs-ups. Emoji can be awarded to [issues](project/issues/index.md), [merge requests](project/merge_requests/index.md),
[snippets](snippets.md), and anywhere you can have a thread. [snippets](snippets.md), and anywhere you can have a thread.
@ -24,8 +19,6 @@ For information on the relevant API, see [Award Emoji API](../api/award_emoji.md
## Sort issues and merge requests on vote count ## Sort issues and merge requests on vote count
> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/2781) in GitLab 8.5.
You can quickly sort issues and merge requests by the number of votes they You can quickly sort issues and merge requests by the number of votes they
have received. The sort options can be found in the dropdown menu as "Most have received. The sort options can be found in the dropdown menu as "Most
popular" and "Least popular". popular" and "Least popular".
@ -38,8 +31,6 @@ downvotes.
## Award emoji for comments ## Award emoji for comments
> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4291) in GitLab 8.9.
Award emoji can also be applied to individual comments when you want to Award emoji can also be applied to individual comments when you want to
celebrate an accomplishment or agree with an opinion. celebrate an accomplishment or agree with an opinion.

View File

@ -139,7 +139,7 @@ the related documentation.
| [Max pipelines per schedule](../../administration/instance_limits.md#limit-the-number-of-pipelines-created-by-a-pipeline-schedule-per-day) | `24` for Free tier, `288` for all paid tiers | Unlimited | | [Max pipelines per schedule](../../administration/instance_limits.md#limit-the-number-of-pipelines-created-by-a-pipeline-schedule-per-day) | `24` for Free tier, `288` for all paid tiers | Unlimited |
| [Scheduled Job Archival](../../user/admin_area/settings/continuous_integration.md#archive-jobs) | 3 months | Never | | [Scheduled Job Archival](../../user/admin_area/settings/continuous_integration.md#archive-jobs) | 3 months | Never |
| Max test cases per [unit test report](../../ci/unit_test_reports.md) | `500_000` | Unlimited | | Max test cases per [unit test report](../../ci/unit_test_reports.md) | `500_000` | Unlimited |
| [Max registered runners](../../administration/instance_limits.md#number-of-registered-runners-per-scope) | `50` per-project and per-group for Free tier,<br/>`1_000` per-group for all paid tiers / `1_000` per-project for all paid tiers | `1_000` per-group / `1_000` per-project | | [Max registered runners](../../administration/instance_limits.md#number-of-registered-runners-per-scope) | Free tier: `50` per-group / `50` per-project <br/> All paid tiers: `1_000` per-group / `1_000` per-project | `1_000` per-group / `1_000` per-project |
## Account and limit settings ## Account and limit settings

View File

@ -156,8 +156,6 @@ to the project:
## Scoped labels **(PREMIUM)** ## Scoped labels **(PREMIUM)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9175) in [GitLab Premium](https://about.gitlab.com/pricing/) 11.10.
Scoped labels allow teams to use the label feature to annotate issues, merge requests Scoped labels allow teams to use the label feature to annotate issues, merge requests
and epics with mutually exclusive labels. This can enable more complicated workflows and epics with mutually exclusive labels. This can enable more complicated workflows
by preventing certain labels from being used together. by preventing certain labels from being used together.
@ -241,14 +239,14 @@ to label notifications for the project only, or the whole group.
## Label priority ## Label priority
> - [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/14189) in GitLab 8.9.
> - Priority sorting is based on the highest priority label only. [This discussion](https://gitlab.com/gitlab-org/gitlab/-/issues/14523) considers changing this.
Labels can have relative priorities, which are used in the **Label priority** and Labels can have relative priorities, which are used in the **Label priority** and
**Priority** sort orders of issues and merge request list pages. Prioritization **Priority** sort orders of issues and merge request list pages. Prioritization
for both group and project labels happens at the project level, and cannot be done for both group and project labels happens at the project level, and cannot be done
from the group label list. from the group label list.
NOTE:
Priority sorting is based on the highest priority label only. [This discussion](https://gitlab.com/gitlab-org/gitlab/-/issues/14523) considers changing this.
From the project label list page, star a label to indicate that it has a priority. From the project label list page, star a label to indicate that it has a priority.
![Labels prioritized](img/labels_prioritized_v13_5.png) ![Labels prioritized](img/labels_prioritized_v13_5.png)

View File

@ -7,8 +7,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w
# To-Do List **(FREE)** # To-Do List **(FREE)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/2817) in GitLab 8.5.
Your *To-Do List* is a chronological list of items waiting for your input. Your *To-Do List* is a chronological list of items waiting for your input.
The items are known as *to-do items*. The items are known as *to-do items*.
@ -67,8 +65,6 @@ You can manually add an item to your To-Do List.
## Create a to-do item by directly addressing someone ## Create a to-do item by directly addressing someone
> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/7926) in GitLab 9.0.
You can create a to-do item by directly addressing someone at the start of a line. You can create a to-do item by directly addressing someone at the start of a line.
For example, in the following comment: For example, in the following comment:

View File

@ -124,7 +124,10 @@ module Gitlab
strong_memoize(:runner_project) do strong_memoize(:runner_project) do
next unless runner&.project_type? next unless runner&.project_type?
projects = runner.projects.take(2) # rubocop: disable CodeReuse/ActiveRecord projects = ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/342147') do
runner.projects.take(2) # rubocop: disable CodeReuse/ActiveRecord
end
projects.first if projects.one? projects.first if projects.one?
end end
end end

View File

@ -0,0 +1,38 @@
# frozen_string_literal: true
module Gitlab
class FeatureCategories
FEATURE_CATEGORY_DEFAULT = 'unknown'
attr_reader :categories
def self.default
@default ||= self.load_from_yaml
end
def self.load_from_yaml
categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml'))
new(categories)
end
def initialize(categories)
@categories = categories.to_set
end
# If valid, returns a feature category from the given request.
def from_request(request)
category = request.headers["HTTP_X_GITLAB_FEATURE_CATEGORY"].presence
return unless category && valid?(category)
return unless ::Gitlab::RequestForgeryProtection.verified?(request.env)
category
end
def valid?(category)
categories.include?(category.to_s)
end
end
end

View File

@ -0,0 +1,15 @@
# frozen_string_literal: true
module Gitlab
module Graphql
module Board
class IssuesConnectionExtension < GraphQL::Schema::Field::ConnectionExtension
def after_resolve(value:, object:, context:, **rest)
::Boards::Issues::ListService
.initialize_relative_positions(object.list.board, context[:current_user], value.nodes)
value
end
end
end
end
end

View File

@ -6,7 +6,7 @@ module Gitlab
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
delegate :to_a, :size, :include?, :empty?, to: :nodes delegate :to_a, :size, :map, :include?, :empty?, to: :nodes
end end
end end
end end

View File

@ -10,7 +10,6 @@ tree:
- labels: - labels:
- :priorities - :priorities
- boards: - boards:
- :milestone
- lists: - lists:
- label: - label:
- :priorities - :priorities

View File

@ -15,7 +15,7 @@ module Gitlab
HEALTH_ENDPOINT = %r{^/-/(liveness|readiness|health|metrics)/?$}.freeze HEALTH_ENDPOINT = %r{^/-/(liveness|readiness|health|metrics)/?$}.freeze
FEATURE_CATEGORY_DEFAULT = 'unknown' FEATURE_CATEGORY_DEFAULT = ::Gitlab::FeatureCategories::FEATURE_CATEGORY_DEFAULT
ENDPOINT_MISSING = 'unknown' ENDPOINT_MISSING = 'unknown'
# These were the top 5 categories at a point in time, chosen as a # These were the top 5 categories at a point in time, chosen as a

View File

@ -57,10 +57,6 @@ module Gitlab
action = "#{controller.action_name}" action = "#{controller.action_name}"
# Try to get the feature category, but don't fail when the controller is
# not an ApplicationController.
feature_category = controller.class.try(:feature_category_for_action, action).to_s
# Devise exposes a method called "request_format" that does the below. # Devise exposes a method called "request_format" that does the below.
# However, this method is not available to all controllers (e.g. certain # However, this method is not available to all controllers (e.g. certain
# Doorkeeper controllers). As such we use the underlying code directly. # Doorkeeper controllers). As such we use the underlying code directly.
@ -91,9 +87,6 @@ module Gitlab
if route if route
path = endpoint_paths_cache[route.request_method][route.path] path = endpoint_paths_cache[route.request_method][route.path]
grape_class = endpoint.options[:for]
feature_category = grape_class.try(:feature_category_for_app, endpoint).to_s
{ controller: 'Grape', action: "#{route.request_method} #{path}", feature_category: feature_category } { controller: 'Grape', action: "#{route.request_method} #{path}", feature_category: feature_category }
end end
end end
@ -109,6 +102,10 @@ module Gitlab
def endpoint_instrumentable_path(raw_path) def endpoint_instrumentable_path(raw_path)
raw_path.sub('(.:format)', '').sub('/:version', '') raw_path.sub('(.:format)', '').sub('/:version', '')
end end
def feature_category
::Gitlab::ApplicationContext.current_context_attribute(:feature_category) || ::Gitlab::FeatureCategories::FEATURE_CATEGORY_DEFAULT
end
end end
end end
end end

View File

@ -6,8 +6,6 @@ module Gitlab
module InOperatorOptimization module InOperatorOptimization
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
class QueryBuilder class QueryBuilder
UnsupportedScopeOrder = Class.new(StandardError)
RECURSIVE_CTE_NAME = 'recursive_keyset_cte' RECURSIVE_CTE_NAME = 'recursive_keyset_cte'
# This class optimizes slow database queries (PostgreSQL specific) where the # This class optimizes slow database queries (PostgreSQL specific) where the
@ -44,14 +42,7 @@ module Gitlab
def initialize(scope:, array_scope:, array_mapping_scope:, finder_query: nil, values: {}) def initialize(scope:, array_scope:, array_mapping_scope:, finder_query: nil, values: {})
@scope, success = Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(scope) @scope, success = Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(scope)
unless success raise(UnsupportedScopeOrder) unless success
error_message = <<~MSG
The order on the scope does not support keyset pagination. You might need to define a custom Order object.\n
See https://docs.gitlab.com/ee/development/database/keyset_pagination.html#complex-order-configuration\n
Or the Gitlab::Pagination::Keyset::Order class for examples
MSG
raise(UnsupportedScopeOrder, error_message)
end
@order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(scope) @order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(scope)
@array_scope = array_scope @array_scope = array_scope

View File

@ -4,11 +4,9 @@ module Gitlab
module Pagination module Pagination
module Keyset module Keyset
class Iterator class Iterator
UnsupportedScopeOrder = Class.new(StandardError)
def initialize(scope:, cursor: {}, use_union_optimization: true, in_operator_optimization_options: nil) def initialize(scope:, cursor: {}, use_union_optimization: true, in_operator_optimization_options: nil)
@scope, success = Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(scope) @scope, success = Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(scope)
raise(UnsupportedScopeOrder, 'The order on the scope does not support keyset pagination') unless success raise(UnsupportedScopeOrder) unless success
@cursor = cursor @cursor = cursor
@order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(scope) @order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(scope)

View File

@ -19,8 +19,6 @@ module Gitlab
FORWARD_DIRECTION = 'n' FORWARD_DIRECTION = 'n'
BACKWARD_DIRECTION = 'p' BACKWARD_DIRECTION = 'p'
UnsupportedScopeOrder = Class.new(StandardError)
# scope - ActiveRecord::Relation object with order by clause # scope - ActiveRecord::Relation object with order by clause
# cursor - Encoded cursor attributes as String. Empty value will requests the first page. # cursor - Encoded cursor attributes as String. Empty value will requests the first page.
# per_page - Number of items per page. # per_page - Number of items per page.
@ -167,7 +165,7 @@ module Gitlab
def build_scope(scope) def build_scope(scope)
keyset_aware_scope, success = Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(scope) keyset_aware_scope, success = Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(scope)
raise(UnsupportedScopeOrder, 'The order on the scope does not support keyset pagination') unless success raise(UnsupportedScopeOrder) unless success
keyset_aware_scope keyset_aware_scope
end end

View File

@ -0,0 +1,19 @@
# frozen_string_literal: true
module Gitlab
module Pagination
module Keyset
class UnsupportedScopeOrder < StandardError
DEFAULT_ERROR_MESSAGE = <<~MSG
The order on the scope does not support keyset pagination. You might need to define a custom Order object.\n
See https://docs.gitlab.com/ee/development/database/keyset_pagination.html#complex-order-configuration\n
Or the Gitlab::Pagination::Keyset::Order class for examples
MSG
def message
DEFAULT_ERROR_MESSAGE
end
end
end
end
end

View File

@ -2,7 +2,7 @@
module QA module QA
RSpec.describe 'Configure', except: { job: 'review-qa-*' } do RSpec.describe 'Configure', except: { job: 'review-qa-*' } do
describe 'Kubernetes Cluster Integration', :requires_admin, :skip_live_env, :smoke do describe 'Kubernetes Cluster Integration', :orchestrated, :requires_admin, :skip_live_env do
context 'Project Clusters' do context 'Project Clusters' do
let!(:cluster) { Service::KubernetesCluster.new(provider_class: Service::ClusterProvider::K3s).create! } let!(:cluster) { Service::KubernetesCluster.new(provider_class: Service::ClusterProvider::K3s).create! }
let(:project) do let(:project) do

View File

@ -38,6 +38,14 @@ RSpec.describe GraphqlController do
sign_in(user) sign_in(user)
end end
it 'sets feature category in ApplicationContext from request' do
request.headers["HTTP_X_GITLAB_FEATURE_CATEGORY"] = "web_ide"
post :execute
expect(::Gitlab::ApplicationContext.current_context_attribute(:feature_category)).to eq('web_ide')
end
it 'returns 200 when user can access API' do it 'returns 200 when user can access API' do
post :execute post :execute

View File

@ -18,6 +18,10 @@ FactoryBot.define do
transient { child_of { nil } } transient { child_of { nil } }
transient { upstream_of { nil } } transient { upstream_of { nil } }
before(:create) do |pipeline, evaluator|
pipeline.ensure_project_iid!
end
after(:build) do |pipeline, evaluator| after(:build) do |pipeline, evaluator|
if evaluator.child_of if evaluator.child_of
pipeline.project = evaluator.child_of.project pipeline.project = evaluator.child_of.project
@ -48,7 +52,6 @@ FactoryBot.define do
transient { ci_ref_presence { true } } transient { ci_ref_presence { true } }
before(:create) do |pipeline, evaluator| before(:create) do |pipeline, evaluator|
pipeline.ensure_project_iid!
pipeline.ensure_ci_ref! if evaluator.ci_ref_presence && pipeline.ci_ref_id.nil? pipeline.ensure_ci_ref! if evaluator.ci_ref_presence && pipeline.ci_ref_id.nil?
end end

View File

@ -31,12 +31,11 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError) end.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
end end
it 'returns issues in the correct order with non-nil relative positions', :aggregate_failures do it 'returns the issues in the correct order' do
# by relative_position and then ID # by relative_position and then ID
result = resolve_board_list_issues result = resolve_board_list_issues
expect(result.map(&:id)).to eq [issue1.id, issue3.id, issue2.id, issue4.id] expect(result.map(&:id)).to eq [issue1.id, issue3.id, issue2.id, issue4.id]
expect(result.map(&:relative_position)).not_to include(nil)
end end
it 'finds only issues matching filters' do it 'finds only issues matching filters' do
@ -119,6 +118,6 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
end end
def resolve_board_list_issues(args: {}, current_user: user) def resolve_board_list_issues(args: {}, current_user: user)
resolve(described_class, obj: list, args: args, ctx: { current_user: current_user }).items resolve(described_class, obj: list, args: args, ctx: { current_user: current_user })
end end
end end

View File

@ -154,6 +154,17 @@ RSpec.describe Types::BaseField do
end end
end end
describe '#resolve' do
context "late_extensions is given" do
it 'registers the late extensions after the regular extensions' do
extension_class = Class.new(GraphQL::Schema::Field::ConnectionExtension)
field = described_class.new(name: 'test', type: GraphQL::Types::String.connection_type, null: true, late_extensions: [extension_class])
expect(field.extensions.last.class).to be(extension_class)
end
end
end
describe '#description' do describe '#description' do
context 'feature flag given' do context 'feature flag given' do
let(:field) { described_class.new(name: 'test', type: GraphQL::Types::String, feature_flag: flag, null: false, description: 'Test description.') } let(:field) { described_class.new(name: 'test', type: GraphQL::Types::String, feature_flag: flag, null: false, description: 'Test description.') }

View File

@ -10,4 +10,12 @@ RSpec.describe GitlabSchema.types['BoardList'] do
expect(described_class).to include_graphql_fields(*expected_fields) expect(described_class).to include_graphql_fields(*expected_fields)
end end
describe 'issues field' do
subject { described_class.fields['issues'] }
it 'has a correct extension' do
is_expected.to have_graphql_extension(Gitlab::Graphql::Board::IssuesConnectionExtension)
end
end
end end

View File

@ -45,4 +45,10 @@ RSpec.describe Gitlab::Database::Partitioning do
described_class.drop_detached_partitions described_class.drop_detached_partitions
end end
end end
context 'ensure that the registered models have partitioning strategy' do
it 'fails when partitioning_strategy is not specified for the model' do
expect(described_class.registered_models).to all(respond_to(:partitioning_strategy))
end
end
end end

View File

@ -0,0 +1,74 @@
# frozen_string_literal: true
require 'fast_spec_helper'
RSpec.describe Gitlab::FeatureCategories do
let(:fake_categories) { %w(foo bar) }
subject { described_class.new(fake_categories) }
describe "#valid?" do
it "returns true if category is known", :aggregate_failures do
expect(subject.valid?('foo')).to be(true)
expect(subject.valid?('zzz')).to be(false)
end
end
describe "#from_request" do
let(:request_env) { {} }
let(:verified) { true }
def fake_request(request_feature_category)
double('request', env: request_env, headers: { "HTTP_X_GITLAB_FEATURE_CATEGORY" => request_feature_category })
end
before do
allow(::Gitlab::RequestForgeryProtection).to receive(:verified?).with(request_env).and_return(verified)
end
it "returns category from request when valid, otherwise returns nil", :aggregate_failures do
expect(subject.from_request(fake_request("foo"))).to be("foo")
expect(subject.from_request(fake_request("zzz"))).to be_nil
end
context "when request is not verified" do
let(:verified) { false }
it "returns nil" do
expect(subject.from_request(fake_request("foo"))).to be_nil
end
end
end
describe "#categories" do
it "returns a set of the given categories" do
expect(subject.categories).to be_a(Set)
expect(subject.categories).to contain_exactly(*fake_categories)
end
end
describe ".load_from_yaml" do
subject { described_class.load_from_yaml }
it "creates FeatureCategories from feature_categories.yml file" do
contents = YAML.load_file(Rails.root.join('config', 'feature_categories.yml'))
expect(subject.categories).to contain_exactly(*contents)
end
end
describe ".default" do
it "returns a memoization of load_from_yaml", :aggregate_failures do
# FeatureCategories.default could have been referenced in another spec, so we need to clean it up here
described_class.instance_variable_set(:@default, nil)
expect(described_class).to receive(:load_from_yaml).once.and_call_original
2.times { described_class.default }
# Uses reference equality to verify memoization
expect(described_class.default).to equal(described_class.default)
expect(described_class.default).to be_a(described_class)
end
end
end

View File

@ -348,7 +348,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do
end end
it 'has every label in config/feature_categories.yml' do it 'has every label in config/feature_categories.yml' do
defaults = [described_class::FEATURE_CATEGORY_DEFAULT, 'not_owned'] defaults = [::Gitlab::FeatureCategories::FEATURE_CATEGORY_DEFAULT, 'not_owned']
feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).map(&:strip) + defaults feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).map(&:strip) + defaults
expect(described_class::FEATURE_CATEGORIES_TO_INITIALIZE).to all(be_in(feature_categories)) expect(described_class::FEATURE_CATEGORIES_TO_INITIALIZE).to all(be_in(feature_categories))

View File

@ -32,7 +32,7 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
it 'measures with correct labels and value' do it 'measures with correct labels and value' do
value = 1 value = 1
expect(prometheus_metric).to receive(metric_method).with({ controller: 'TestController', action: 'show', feature_category: '' }, value) expect(prometheus_metric).to receive(metric_method).with({ controller: 'TestController', action: 'show', feature_category: ::Gitlab::FeatureCategories::FEATURE_CATEGORY_DEFAULT }, value)
transaction.send(metric_method, :bau, value) transaction.send(metric_method, :bau, value)
end end
@ -105,6 +105,9 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
namespace: "/projects") namespace: "/projects")
env['api.endpoint'] = endpoint env['api.endpoint'] = endpoint
# This is needed since we're not actually making a request, which would trigger the controller pushing to the context
::Gitlab::ApplicationContext.push(feature_category: 'projects')
end end
it 'provides labels with the method and path of the route in the grape endpoint' do it 'provides labels with the method and path of the route in the grape endpoint' do
@ -129,7 +132,7 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
include_context 'ActionController request' include_context 'ActionController request'
it 'tags a transaction with the name and action of a controller' do it 'tags a transaction with the name and action of a controller' do
expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: '' }) expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: ::Gitlab::FeatureCategories::FEATURE_CATEGORY_DEFAULT })
end end
it 'contains only the labels defined for transactions' do it 'contains only the labels defined for transactions' do
@ -140,7 +143,7 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
let(:request) { double(:request, format: double(:format, ref: :json)) } let(:request) { double(:request, format: double(:format, ref: :json)) }
it 'appends the mime type to the transaction action' do it 'appends the mime type to the transaction action' do
expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json', feature_category: '' }) expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json', feature_category: ::Gitlab::FeatureCategories::FEATURE_CATEGORY_DEFAULT })
end end
end end
@ -148,13 +151,15 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
let(:request) { double(:request, format: double(:format, ref: 'http://example.com')) } let(:request) { double(:request, format: double(:format, ref: 'http://example.com')) }
it 'does not append the MIME type to the transaction action' do it 'does not append the MIME type to the transaction action' do
expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: '' }) expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: ::Gitlab::FeatureCategories::FEATURE_CATEGORY_DEFAULT })
end end
end end
context 'when the feature category is known' do context 'when the feature category is known' do
it 'includes it in the feature category label' do it 'includes it in the feature category label' do
expect(controller_class).to receive(:feature_category_for_action).with('show').and_return(:source_code_management) # This is needed since we're not actually making a request, which would trigger the controller pushing to the context
::Gitlab::ApplicationContext.push(feature_category: 'source_code_management')
expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: "source_code_management" }) expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: "source_code_management" })
end end
end end

View File

@ -30,7 +30,7 @@ RSpec.describe 'get board lists' do
nodes { nodes {
lists { lists {
nodes { nodes {
issues(filters: {labelName: "#{label2.title}"}) { issues(filters: {labelName: "#{label2.title}"}, first: 3) {
count count
nodes { nodes {
#{all_graphql_fields_for('issues'.classify)} #{all_graphql_fields_for('issues'.classify)}
@ -44,6 +44,10 @@ RSpec.describe 'get board lists' do
) )
end end
def issue_id
issues_data.map { |i| i['id'] }
end
def issue_titles def issue_titles
issues_data.map { |i| i['title'] } issues_data.map { |i| i['title'] }
end end
@ -60,6 +64,7 @@ RSpec.describe 'get board lists' do
let!(:issue3) { create(:issue, project: issue_project, labels: [label, label2], relative_position: nil) } let!(:issue3) { create(:issue, project: issue_project, labels: [label, label2], relative_position: nil) }
let!(:issue4) { create(:issue, project: issue_project, labels: [label], relative_position: 9) } let!(:issue4) { create(:issue, project: issue_project, labels: [label], relative_position: 9) }
let!(:issue5) { create(:issue, project: issue_project, labels: [label2], relative_position: 432) } let!(:issue5) { create(:issue, project: issue_project, labels: [label2], relative_position: 432) }
let!(:issue6) { create(:issue, project: issue_project, labels: [label, label2], relative_position: nil) }
context 'when the user does not have access to the board' do context 'when the user does not have access to the board' do
it 'returns nil' do it 'returns nil' do
@ -72,14 +77,19 @@ RSpec.describe 'get board lists' do
context 'when user can read the board' do context 'when user can read the board' do
before do before do
board_parent.add_reporter(user) board_parent.add_reporter(user)
post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user)
end end
it 'can access the issues', :aggregate_failures do it 'can access the issues', :aggregate_failures do
post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user) # ties for relative positions are broken by id in ascending order by default
expect(issue_titles).to eq([issue2.title, issue1.title, issue3.title]) expect(issue_titles).to eq([issue2.title, issue1.title, issue3.title])
expect(issue_relative_positions).not_to include(nil) expect(issue_relative_positions).not_to include(nil)
end end
it 'does not set the relative positions of the issues not being returned', :aggregate_failures do
expect(issue_id).not_to include(issue6.id)
expect(issue3.relative_position).to be_nil
end
end end
end end

View File

@ -87,19 +87,25 @@ module Ci
end end
context 'for specific runner' do context 'for specific runner' do
context 'with FF disabled' do context 'with tables decoupling disabled' do
before do before do
stub_feature_flags( stub_feature_flags(
ci_pending_builds_project_runners_decoupling: false, ci_pending_builds_project_runners_decoupling: false,
ci_queueing_builds_enabled_checks: false) ci_queueing_builds_enabled_checks: false)
end end
around do |example|
allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
example.run
end
end
it 'does not pick a build' do it 'does not pick a build' do
expect(execute(specific_runner)).to be_nil expect(execute(specific_runner)).to be_nil
end end
end end
context 'with FF enabled' do context 'with tables decoupling enabled' do
before do before do
stub_feature_flags( stub_feature_flags(
ci_pending_builds_project_runners_decoupling: true, ci_pending_builds_project_runners_decoupling: true,
@ -266,17 +272,23 @@ module Ci
context 'and uses project runner' do context 'and uses project runner' do
let(:build) { execute(specific_runner) } let(:build) { execute(specific_runner) }
context 'with FF disabled' do context 'with tables decoupling disabled' do
before do before do
stub_feature_flags( stub_feature_flags(
ci_pending_builds_project_runners_decoupling: false, ci_pending_builds_project_runners_decoupling: false,
ci_queueing_builds_enabled_checks: false) ci_queueing_builds_enabled_checks: false)
end end
around do |example|
allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
example.run
end
end
it { expect(build).to be_nil } it { expect(build).to be_nil }
end end
context 'with FF enabled' do context 'with tables decoupling enabled' do
before do before do
stub_feature_flags( stub_feature_flags(
ci_pending_builds_project_runners_decoupling: true, ci_pending_builds_project_runners_decoupling: true,
@ -791,6 +803,12 @@ module Ci
stub_feature_flags(ci_queueing_denormalize_shared_runners_information: false) stub_feature_flags(ci_queueing_denormalize_shared_runners_information: false)
end end
around do |example|
allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
example.run
end
end
include_examples 'handles runner assignment' include_examples 'handles runner assignment'
end end
@ -807,6 +825,12 @@ module Ci
stub_feature_flags(ci_queueing_denormalize_tags_information: false) stub_feature_flags(ci_queueing_denormalize_tags_information: false)
end end
around do |example|
allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
example.run
end
end
include_examples 'handles runner assignment' include_examples 'handles runner assignment'
end end
@ -815,6 +839,12 @@ module Ci
stub_feature_flags(ci_queueing_denormalize_namespace_traversal_ids: false) stub_feature_flags(ci_queueing_denormalize_namespace_traversal_ids: false)
end end
around do |example|
allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
example.run
end
end
include_examples 'handles runner assignment' include_examples 'handles runner assignment'
end end
end end
@ -824,6 +854,12 @@ module Ci
stub_feature_flags(ci_pending_builds_queue_source: false) stub_feature_flags(ci_pending_builds_queue_source: false)
end end
around do |example|
allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
example.run
end
end
include_examples 'handles runner assignment' include_examples 'handles runner assignment'
end end
end end

File diff suppressed because it is too large Load Diff

View File

@ -81,7 +81,6 @@
- "./spec/presenters/ci/build_runner_presenter_spec.rb" - "./spec/presenters/ci/build_runner_presenter_spec.rb"
- "./spec/presenters/ci/pipeline_presenter_spec.rb" - "./spec/presenters/ci/pipeline_presenter_spec.rb"
- "./spec/presenters/packages/detail/package_presenter_spec.rb" - "./spec/presenters/packages/detail/package_presenter_spec.rb"
- "./spec/requests/api/ci/runner/jobs_request_post_spec.rb"
- "./spec/requests/api/ci/runner/runners_post_spec.rb" - "./spec/requests/api/ci/runner/runners_post_spec.rb"
- "./spec/requests/api/ci/runners_spec.rb" - "./spec/requests/api/ci/runners_spec.rb"
- "./spec/requests/api/commit_statuses_spec.rb" - "./spec/requests/api/commit_statuses_spec.rb"
@ -105,7 +104,6 @@
- "./spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb" - "./spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb"
- "./spec/services/ci/job_artifacts/destroy_associations_service_spec.rb" - "./spec/services/ci/job_artifacts/destroy_associations_service_spec.rb"
- "./spec/services/ci/job_artifacts/destroy_batch_service_spec.rb" - "./spec/services/ci/job_artifacts/destroy_batch_service_spec.rb"
- "./spec/services/ci/register_job_service_spec.rb"
- "./spec/services/deployments/older_deployments_drop_service_spec.rb" - "./spec/services/deployments/older_deployments_drop_service_spec.rb"
- "./spec/services/environments/stop_service_spec.rb" - "./spec/services/environments/stop_service_spec.rb"
- "./spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb" - "./spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb"

View File

@ -33,8 +33,10 @@ module Database
end end
def cleanup_with_cross_database_modification_prevented def cleanup_with_cross_database_modification_prevented
ActiveSupport::Notifications.unsubscribe(PreventCrossDatabaseModification.cross_database_context[:subscriber]) if PreventCrossDatabaseModification.cross_database_context
PreventCrossDatabaseModification.cross_database_context[:enabled] = false ActiveSupport::Notifications.unsubscribe(PreventCrossDatabaseModification.cross_database_context[:subscriber])
PreventCrossDatabaseModification.cross_database_context[:enabled] = false
end
end end
end end
@ -55,8 +57,11 @@ module Database
end end
def self.prevent_cross_database_modification!(connection, sql) def self.prevent_cross_database_modification!(connection, sql)
return unless cross_database_context
return unless cross_database_context[:enabled] return unless cross_database_context[:enabled]
return if connection.pool.instance_of?(ActiveRecord::ConnectionAdapters::NullPool)
database = connection.pool.db_config.name database = connection.pool.db_config.name
if sql.start_with?('SAVEPOINT') if sql.start_with?('SAVEPOINT')
@ -87,7 +92,7 @@ module Database
if schemas.many? if schemas.many?
raise Database::PreventCrossDatabaseModification::CrossDatabaseModificationAcrossUnsupportedTablesError, raise Database::PreventCrossDatabaseModification::CrossDatabaseModificationAcrossUnsupportedTablesError,
"Cross-database data modification of '#{schemas.to_a.join(", ")}' were detected within " \ "Cross-database data modification of '#{schemas.to_a.join(", ")}' were detected within " \
"a transaction modifying the '#{all_tables.to_a.join(", ")}'" "a transaction modifying the '#{all_tables.to_a.join(", ")}' tables"
end end
end end
end end
@ -96,16 +101,20 @@ end
Gitlab::Database.singleton_class.prepend( Gitlab::Database.singleton_class.prepend(
Database::PreventCrossDatabaseModification::GitlabDatabaseMixin) Database::PreventCrossDatabaseModification::GitlabDatabaseMixin)
CROSS_DB_MODIFICATION_ALLOW_LIST = Set.new(YAML.load_file(File.join(__dir__, 'cross-database-modification-allowlist.yml'))).freeze
RSpec.configure do |config| RSpec.configure do |config|
config.include(::Database::PreventCrossDatabaseModification::SpecHelpers) config.include(::Database::PreventCrossDatabaseModification::SpecHelpers)
# Using before and after blocks because the around block causes problems with the let_it_be # Using before and after blocks because the around block causes problems with the let_it_be
# record creations. It makes an extra savepoint which breaks the transaction count logic. # record creations. It makes an extra savepoint which breaks the transaction count logic.
config.before(:each, :prevent_cross_database_modification) do config.before do |example_file|
with_cross_database_modification_prevented if CROSS_DB_MODIFICATION_ALLOW_LIST.exclude?(example_file.file_path)
with_cross_database_modification_prevented
end
end end
config.after(:each, :prevent_cross_database_modification) do config.after do |example_file|
cleanup_with_cross_database_modification_prevented cleanup_with_cross_database_modification_prevented
end end
end end

View File

@ -64,6 +64,10 @@ module Database
ensure ensure
ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber
end end
def allow_cross_joins_across_databases(url:, &block)
::Gitlab::Database.allow_cross_joins_across_databases(url: url, &block)
end
end end
module GitlabDatabaseMixin module GitlabDatabaseMixin

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'a connection with collection methods' do RSpec.shared_examples 'a connection with collection methods' do
%i[to_a size include? empty?].each do |method_name| %i[to_a size map include? empty?].each do |method_name|
it "responds to #{method_name}" do it "responds to #{method_name}" do
expect(connection).to respond_to(method_name) expect(connection).to respond_to(method_name)
end end

View File

@ -56,6 +56,7 @@ const (
apiPattern = `^/api/` apiPattern = `^/api/`
ciAPIPattern = `^/ci/api/` ciAPIPattern = `^/ci/api/`
gitProjectPattern = `^/.+\.git/` gitProjectPattern = `^/.+\.git/`
geoGitProjectPattern = `^/[^-].+\.git/` // Prevent matching routes like /-/push_from_secondary
projectPattern = `^/([^/]+/){1,}[^/]+/` projectPattern = `^/([^/]+/){1,}[^/]+/`
apiProjectPattern = apiPattern + `v4/projects/[^/]+/` // API: Projects can be encoded via group%2Fsubgroup%2Fproject apiProjectPattern = apiPattern + `v4/projects/[^/]+/` // API: Projects can be encoded via group%2Fsubgroup%2Fproject
snippetUploadPattern = `^/uploads/personal_snippet` snippetUploadPattern = `^/uploads/personal_snippet`
@ -348,10 +349,9 @@ func configureRoutes(u *upstream) {
// pulls are performed against a different source of truth. Ideally, we'd // pulls are performed against a different source of truth. Ideally, we'd
// proxy/redirect pulls as well, when the secondary is not up-to-date. // proxy/redirect pulls as well, when the secondary is not up-to-date.
// //
u.route("GET", gitProjectPattern+`info/refs\z`, git.GetInfoRefsHandler(api)), u.route("GET", geoGitProjectPattern+`info/refs\z`, git.GetInfoRefsHandler(api)),
u.route("POST", gitProjectPattern+`git-upload-pack\z`, contentEncodingHandler(git.UploadPack(api)), withMatcher(isContentType("application/x-git-upload-pack-request"))), u.route("POST", geoGitProjectPattern+`git-upload-pack\z`, contentEncodingHandler(git.UploadPack(api)), withMatcher(isContentType("application/x-git-upload-pack-request"))),
u.route("POST", gitProjectPattern+`git-receive-pack\z`, contentEncodingHandler(git.ReceivePack(api)), withMatcher(isContentType("application/x-git-receive-pack-request"))), u.route("GET", geoGitProjectPattern+`gitlab-lfs/objects/([0-9a-f]{64})\z`, defaultUpstream),
u.route("PUT", gitProjectPattern+`gitlab-lfs/objects/([0-9a-f]{64})/([0-9]+)\z`, lfs.PutStore(api, signingProxy, preparers.lfs), withMatcher(isContentType("application/octet-stream"))),
// Serve health checks from this Geo secondary // Serve health checks from this Geo secondary
u.route("", "^/-/(readiness|liveness)$", static.DeployPage(probeUpstream)), u.route("", "^/-/(readiness|liveness)$", static.DeployPage(probeUpstream)),

View File

@ -99,6 +99,8 @@ func TestGeoProxyFeatureEnabledOnGeoSecondarySite(t *testing.T) {
defer wsDeferredClose() defer wsDeferredClose()
testCases := []testCase{ testCases := []testCase{
{"push from secondary is forwarded", "/-/push_from_secondary/foo/bar.git/info/refs", "Geo primary received request to path /-/push_from_secondary/foo/bar.git/info/refs"},
{"LFS files are served locally", "/group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6", "Local Rails server received request to path /group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6"},
{"jobs request is forwarded", "/api/v4/jobs/request", "Geo primary received request to path /api/v4/jobs/request"}, {"jobs request is forwarded", "/api/v4/jobs/request", "Geo primary received request to path /api/v4/jobs/request"},
{"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"}, {"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"},
{"unknown route is forwarded", "/anything", "Geo primary received request to path /anything"}, {"unknown route is forwarded", "/anything", "Geo primary received request to path /anything"},
@ -117,6 +119,7 @@ func TestGeoProxyFeatureDisabledOnNonGeoSecondarySite(t *testing.T) {
defer wsDeferredClose() defer wsDeferredClose()
testCases := []testCase{ testCases := []testCase{
{"LFS files are served locally", "/group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6", "Local Rails server received request to path /group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6"},
{"jobs request is served locally", "/api/v4/jobs/request", "Local Rails server received request to path /api/v4/jobs/request"}, {"jobs request is served locally", "/api/v4/jobs/request", "Local Rails server received request to path /api/v4/jobs/request"},
{"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"}, {"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"},
{"unknown route is served locally", "/anything", "Local Rails server received request to path /anything"}, {"unknown route is served locally", "/anything", "Local Rails server received request to path /anything"},
@ -134,6 +137,7 @@ func TestGeoProxyFeatureEnabledOnNonGeoSecondarySite(t *testing.T) {
defer wsDeferredClose() defer wsDeferredClose()
testCases := []testCase{ testCases := []testCase{
{"LFS files are served locally", "/group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6", "Local Rails server received request to path /group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6"},
{"jobs request is served locally", "/api/v4/jobs/request", "Local Rails server received request to path /api/v4/jobs/request"}, {"jobs request is served locally", "/api/v4/jobs/request", "Local Rails server received request to path /api/v4/jobs/request"},
{"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"}, {"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"},
{"unknown route is served locally", "/anything", "Local Rails server received request to path /anything"}, {"unknown route is served locally", "/anything", "Local Rails server received request to path /anything"},
@ -151,6 +155,7 @@ func TestGeoProxyFeatureEnabledButWithAPIError(t *testing.T) {
defer wsDeferredClose() defer wsDeferredClose()
testCases := []testCase{ testCases := []testCase{
{"LFS files are served locally", "/group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6", "Local Rails server received request to path /group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6"},
{"jobs request is served locally", "/api/v4/jobs/request", "Local Rails server received request to path /api/v4/jobs/request"}, {"jobs request is served locally", "/api/v4/jobs/request", "Local Rails server received request to path /api/v4/jobs/request"},
{"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"}, {"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"},
{"unknown route is served locally", "/anything", "Local Rails server received request to path /anything"}, {"unknown route is served locally", "/anything", "Local Rails server received request to path /anything"},
@ -174,12 +179,15 @@ func TestGeoProxyFeatureEnablingAndDisabling(t *testing.T) {
defer wsDeferredClose() defer wsDeferredClose()
testCasesLocal := []testCase{ testCasesLocal := []testCase{
{"LFS files are served locally", "/group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6", "Local Rails server received request to path /group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6"},
{"jobs request is served locally", "/api/v4/jobs/request", "Local Rails server received request to path /api/v4/jobs/request"}, {"jobs request is served locally", "/api/v4/jobs/request", "Local Rails server received request to path /api/v4/jobs/request"},
{"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"}, {"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"},
{"unknown route is served locally", "/anything", "Local Rails server received request to path /anything"}, {"unknown route is served locally", "/anything", "Local Rails server received request to path /anything"},
} }
testCasesProxied := []testCase{ testCasesProxied := []testCase{
{"push from secondary is forwarded", "/-/push_from_secondary/foo/bar.git/info/refs", "Geo primary received request to path /-/push_from_secondary/foo/bar.git/info/refs"},
{"LFS files are served locally", "/group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6", "Local Rails server received request to path /group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6"},
{"jobs request is forwarded", "/api/v4/jobs/request", "Geo primary received request to path /api/v4/jobs/request"}, {"jobs request is forwarded", "/api/v4/jobs/request", "Geo primary received request to path /api/v4/jobs/request"},
{"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"}, {"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"},
{"unknown route is forwarded", "/anything", "Geo primary received request to path /anything"}, {"unknown route is forwarded", "/anything", "Geo primary received request to path /anything"},