From 9e7d45afd74a71be22c2413f4857d4389e360a42 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 3 Oct 2023 06:09:20 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- Gemfile | 2 +- Gemfile.checksum | 3 +- Gemfile.lock | 4 +- .../merge_requests/_description.html.haml | 2 +- .../merge_requests/_mr_title.html.haml | 4 +- app/views/shared/_broadcast_message.html.haml | 4 +- .../empty_states/_merge_requests.html.haml | 2 +- .../gitlab_main_clusterwide.yaml | 4 + ...e_index_events_author_id_and_created_at.rb | 18 ++++ db/schema_migrations/20230726172100 | 1 + doc/ci/secrets/convert-to-id-tokens.md | 19 ++--- doc/ci/secrets/id_token_authentication.md | 17 ---- doc/ci/yaml/index.md | 6 +- lib/gitlab/database/gitlab_schema.rb | 35 +++++--- lib/gitlab/database/gitlab_schema_info.rb | 72 +++++++++++++++- .../prevent_cross_database_modification.rb | 2 +- qa/gdk/Dockerfile.gdk | 2 +- qa/qa/page/merge_request/index.rb | 4 +- qa/qa/page/merge_request/show.rb | 22 ++--- qa/qa/page/project/web_ide/vscode.rb | 10 +-- .../lib/gitlab/database/gitlab_schema_spec.rb | 82 ++++++++++++------- .../database/no_cross_db_foreign_keys_spec.rb | 6 +- spec/models/application_setting_spec.rb | 38 +++------ .../container_expiration_policy_spec.rb | 3 +- .../image_ttl_group_policy_spec.rb | 3 +- .../integrations/apple_app_store_spec.rb | 3 +- spec/models/integrations/google_play_spec.rb | 3 +- spec/models/issue_spec.rb | 3 +- spec/models/namespace/package_setting_spec.rb | 10 +-- spec/models/project_setting_spec.rb | 3 +- spec/models/user_preference_spec.rb | 3 +- spec/requests/api/groups_spec.rb | 14 ++-- spec/support/database/prevent_cross_joins.rb | 2 +- 33 files changed, 246 insertions(+), 160 deletions(-) create mode 100644 db/post_migrate/20230726172100_remove_index_events_author_id_and_created_at.rb create mode 100644 db/schema_migrations/20230726172100 diff --git a/Gemfile b/Gemfile index e9df0507de9..1719d98e570 100644 --- a/Gemfile +++ b/Gemfile @@ -195,7 +195,7 @@ gem 'seed-fu', '~> 2.3.7' # rubocop:todo Gemfile/MissingFeatureCategory gem 'elasticsearch-model', '~> 7.2' # rubocop:todo Gemfile/MissingFeatureCategory gem 'elasticsearch-rails', '~> 7.2', require: 'elasticsearch/rails/instrumentation' # rubocop:todo Gemfile/MissingFeatureCategory gem 'elasticsearch-api', '7.13.3' # rubocop:todo Gemfile/MissingFeatureCategory -gem 'aws-sdk-core', '~> 3.184.0' # rubocop:todo Gemfile/MissingFeatureCategory +gem 'aws-sdk-core', '~> 3.185.0' # rubocop:todo Gemfile/MissingFeatureCategory gem 'aws-sdk-cloudformation', '~> 1' # rubocop:todo Gemfile/MissingFeatureCategory gem 'aws-sdk-s3', '~> 1.136.0' # rubocop:todo Gemfile/MissingFeatureCategory gem 'faraday_middleware-aws-sigv4', '~>0.3.0' # rubocop:todo Gemfile/MissingFeatureCategory diff --git a/Gemfile.checksum b/Gemfile.checksum index 7d67e20d41f..adbf9172b9e 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -36,7 +36,7 @@ {"name":"aws-eventstream","version":"1.2.0","platform":"ruby","checksum":"ffa53482c92880b001ff2fb06919b9bb82fd847cbb0fa244985d2ebb6dd0d1df"}, {"name":"aws-partitions","version":"1.761.0","platform":"ruby","checksum":"291e444e1edfc92c5521a6dbdd1236ccc3f122b3520163b2be6ec5b6ef350ef2"}, {"name":"aws-sdk-cloudformation","version":"1.41.0","platform":"ruby","checksum":"31e47539719734413671edf9b1a31f8673fbf9688549f50c41affabbcb1c6b26"}, -{"name":"aws-sdk-core","version":"3.184.0","platform":"ruby","checksum":"408cffb9716f338f08998ea069bac1491cb8f6222f07f32679f0ec7b8b04e4f9"}, +{"name":"aws-sdk-core","version":"3.185.0","platform":"ruby","checksum":"2c8fe9a952bd8423181734dfcb4138ce48c7675bee365036b1577c110e70db03"}, {"name":"aws-sdk-kms","version":"1.64.0","platform":"ruby","checksum":"40de596c95047bfc6e1aacea24f3df6241aa716b6f7ce08ac4c5f7e3120395ad"}, {"name":"aws-sdk-s3","version":"1.136.0","platform":"ruby","checksum":"3547302a85d51de6cc75b48fb37d328f65f6526e7fc73a27a5b1b871f99a8d63"}, {"name":"aws-sigv4","version":"1.6.0","platform":"ruby","checksum":"ca9e6a15cd424f1f32b524b9760995331459bc22e67d3daad4fcf0c0084b087d"}, @@ -489,7 +489,6 @@ {"name":"rbtrace","version":"0.4.14","platform":"ruby","checksum":"162bbf89cecabfc4f09c869b655f6f3a679c4870ebb7cbdcadf7393a81cc1769"}, {"name":"rbtree","version":"0.4.6","platform":"ruby","checksum":"14eea4469b24fd2472542e5f3eb105d6344c8ccf36f0b56d55fdcfeb4e0f10fc"}, {"name":"rchardet","version":"1.8.0","platform":"ruby","checksum":"693acd5253d5ade81a51940697955f6dd4bb2f0d245bda76a8e23deec70a52c7"}, -{"name":"rdoc","version":"6.3.2","platform":"ruby","checksum":"def4a720235c27d56c176ae73555e647eb04ea58a8bbaa927f8f9f79de7805a6"}, {"name":"re2","version":"2.1.3","platform":"aarch64-linux","checksum":"27316bb47cfc0f28cfd1626426120e1c55ca8420a64c9e966f8feb1c911eae2a"}, {"name":"re2","version":"2.1.3","platform":"arm-linux","checksum":"81ffdd76b202f24461b4868abed96c994e2106e57970004b841499da983f688c"}, {"name":"re2","version":"2.1.3","platform":"arm64-darwin","checksum":"86d553e85779943a353865cbfdd89156c0411b92a1c7fe6abf1024135d53190e"}, diff --git a/Gemfile.lock b/Gemfile.lock index 21907a0c9c8..988194ea5ca 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -270,7 +270,7 @@ GEM aws-sdk-cloudformation (1.41.0) aws-sdk-core (~> 3, >= 3.99.0) aws-sigv4 (~> 1.1) - aws-sdk-core (3.184.0) + aws-sdk-core (3.185.0) aws-eventstream (~> 1, >= 1.0.2) aws-partitions (~> 1, >= 1.651.0) aws-sigv4 (~> 1.5) @@ -1746,7 +1746,7 @@ DEPENDENCIES autoprefixer-rails (= 10.2.5.1) awesome_print aws-sdk-cloudformation (~> 1) - aws-sdk-core (~> 3.184.0) + aws-sdk-core (~> 3.185.0) aws-sdk-s3 (~> 1.136.0) axe-core-rspec babosa (~> 2.0) diff --git a/app/views/projects/merge_requests/_description.html.haml b/app/views/projects/merge_requests/_description.html.haml index 5590f9e6184..89eed0789e8 100644 --- a/app/views/projects/merge_requests/_description.html.haml +++ b/app/views/projects/merge_requests/_description.html.haml @@ -1,6 +1,6 @@ %div - if @merge_request.description.present? - .description{ class: ['gl-mt-4!', can?(current_user, :update_merge_request, @merge_request) ? 'js-task-list-container' : ''], data: { qa_selector: 'description_content' } } + .description{ class: ['gl-mt-4!', can?(current_user, :update_merge_request, @merge_request) ? 'js-task-list-container' : ''], data: { testid: 'description-content' } } .md = markdown_field(@merge_request, :description) %textarea.hidden.js-task-list-field{ data: { value: @merge_request.description } } diff --git a/app/views/projects/merge_requests/_mr_title.html.haml b/app/views/projects/merge_requests/_mr_title.html.haml index 43bd6c66f74..6204e18c533 100644 --- a/app/views/projects/merge_requests/_mr_title.html.haml +++ b/app/views/projects/merge_requests/_mr_title.html.haml @@ -14,7 +14,7 @@ .detail-page-header.border-bottom-0.gl-display-block.gl-pt-5{ class: "gl-md-display-flex! #{'is-merge-request' if moved_mr_sidebar_enabled? && !fluid_layout}" } .detail-page-header-body - %h1.title.page-title.gl-font-size-h-display.gl-my-0.gl-display-inline-block{ data: { qa_selector: 'title_content' } } + %h1.title.page-title.gl-font-size-h-display.gl-my-0.gl-display-inline-block{ data: { testid: 'title-content' } } = markdown_field(@merge_request, :title) - unless hide_gutter_toggle @@ -24,7 +24,7 @@ .detail-page-header-actions.gl-align-self-start.is-merge-request.js-issuable-actions.gl-display-flex - if can_update_merge_request - = render Pajamas::ButtonComponent.new(href: edit_project_merge_request_path(@project, @merge_request), button_options: {class: "gl-display-none gl-md-display-block js-issuable-edit", data: { qa_selector: "edit_title_button" }}) do + = render Pajamas::ButtonComponent.new(href: edit_project_merge_request_path(@project, @merge_request), button_options: {class: "gl-display-none gl-md-display-block js-issuable-edit", data: { testid: "edit-title-button" }}) do = _('Edit') - if @merge_request.source_project diff --git a/app/views/shared/_broadcast_message.html.haml b/app/views/shared/_broadcast_message.html.haml index 2f470d5ef53..12571ef5b73 100644 --- a/app/views/shared/_broadcast_message.html.haml +++ b/app/views/shared/_broadcast_message.html.haml @@ -24,7 +24,7 @@ - else - notification_class = "js-broadcast-notification-#{message.id}" - notification_class << ' preview' if preview - .gl-broadcast-message.broadcast-notification-message.gl-mt-3{ role: "alert", class: notification_class, data: { qa_selector: 'broadcast_notification_container' } } + .gl-broadcast-message.broadcast-notification-message.gl-mt-3{ role: "alert", class: notification_class, data: { testid: 'broadcast-notification-container' } } .gl-broadcast-message-content .gl-broadcast-message-icon = sprite_icon(icon_name, css_class: 'vertical-align-text-top') @@ -38,4 +38,4 @@ = render Pajamas::ButtonComponent.new(category: :tertiary, icon: 'close', size: :small, - button_options: { class: 'js-dismiss-current-broadcast-notification', 'aria-label': _('Close'), data: { id: message.id, expire_date: message.ends_at.iso8601, qa_selector: 'close_button' } }) + button_options: { class: 'js-dismiss-current-broadcast-notification', 'aria-label': _('Close'), data: { id: message.id, expire_date: message.ends_at.iso8601, testid: 'close-button' } }) diff --git a/app/views/shared/empty_states/_merge_requests.html.haml b/app/views/shared/empty_states/_merge_requests.html.haml index 3622b97d53e..5a96b51be61 100644 --- a/app/views/shared/empty_states/_merge_requests.html.haml +++ b/app/views/shared/empty_states/_merge_requests.html.haml @@ -37,4 +37,4 @@ = _("Interested parties can even contribute by pushing commits if they want to.") - if button_path .text-center - = link_button_to _('New merge request'), button_path, title: _('New merge request'), id: 'new_merge_request_link', data: { qa_selector: "new_merge_request_button" }, variant: :confirm + = link_button_to _('New merge request'), button_path, title: _('New merge request'), id: 'new_merge_request_link', data: { testid: "new-merge-request-button" }, variant: :confirm diff --git a/db/gitlab_schemas/gitlab_main_clusterwide.yaml b/db/gitlab_schemas/gitlab_main_clusterwide.yaml index 7d1de57b687..8f07f0caf4f 100644 --- a/db/gitlab_schemas/gitlab_main_clusterwide.yaml +++ b/db/gitlab_schemas/gitlab_main_clusterwide.yaml @@ -11,6 +11,10 @@ allow_cross_transactions: # temporarily allow cross-transaction between clusterwide till all tables # are moved to either _clusterwide or _cell - gitlab_main + # Temporarily allow cross-transaction with cell + # until offenses for each table is resolved. + - gitlab_main_cell: + specific_tables: [] allow_cross_foreign_keys: # temporarily allow FKs between clusterwide till all tables # are moved to either _clusterwide or _cell diff --git a/db/post_migrate/20230726172100_remove_index_events_author_id_and_created_at.rb b/db/post_migrate/20230726172100_remove_index_events_author_id_and_created_at.rb new file mode 100644 index 00000000000..ee426a11342 --- /dev/null +++ b/db/post_migrate/20230726172100_remove_index_events_author_id_and_created_at.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class RemoveIndexEventsAuthorIdAndCreatedAt < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'index_events_on_author_id_and_created_at_merge_requests' + + # TODO: Index to be destroyed synchronously in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127657 + def up + prepare_async_index_removal :events, + [:author_id, :created_at], + name: INDEX_NAME + end + + def down + unprepare_async_index :events, + [:author_id, :created_at], + name: INDEX_NAME + end +end diff --git a/db/schema_migrations/20230726172100 b/db/schema_migrations/20230726172100 new file mode 100644 index 00000000000..e01e4357185 --- /dev/null +++ b/db/schema_migrations/20230726172100 @@ -0,0 +1 @@ +f821d06c66663b10f113cafd97731f8938e85b84581065a9726b74f3dc2ec5ef \ No newline at end of file diff --git a/doc/ci/secrets/convert-to-id-tokens.md b/doc/ci/secrets/convert-to-id-tokens.md index d2c9a55da1f..a477b73c107 100644 --- a/doc/ci/secrets/convert-to-id-tokens.md +++ b/doc/ci/secrets/convert-to-id-tokens.md @@ -9,18 +9,13 @@ type: tutorial This tutorial demonstrates how to convert your existing CI/CD secrets configuration to use [ID Tokens](../secrets/id_token_authentication.md). -The `CI_JOB_JWT` variables are deprecated, but updating to ID tokens requires some important configuration changes to work with Vault. If you have more than a handful of jobs, converting everything at once is a daunting task. +The `CI_JOB_JWT` variables are deprecated, but updating to ID tokens requires some +important configuration changes to work with Vault. If you have more than a handful of jobs, +converting everything at once is a daunting task. -From GitLab 15.9 to 15.11, [enable the automatic ID token authentication](../secrets/id_token_authentication.md#enable-automatic-id-token-authentication-deprecated) -setting to enable ID Tokens and disable `CI_JOB_JWT` tokens. - -In GitLab 16.0 and later you can use ID tokens without any settings changes. -Jobs that use `secrets:vault` automatically do not have `CI_JOB_JWT` tokens available, -Jobs that don't use `secrets:vault` can still use `CI_JOB_JWT` tokens. - -This tutorial will focus on v16 onward, if you are running a slightly older version you will need to toggle the `Limit JSON Web Token (JWT) access` setting as appropriate. - -There isn't one standard method to migrate to [ID tokens](../secrets/id_token_authentication.md), so this tutorial includes two variations for how to convert your existing CI/CD secrets. Choose the method that is most appropriate for your use case: +There isn't one standard method to migrate to [ID tokens](../secrets/id_token_authentication.md), so this tutorial +includes two variations for how to convert your existing CI/CD secrets. Choose the method that is most appropriate for +your use case: 1. Update your Vault configuration: - Method A: Migrate JWT roles to the new Vault auth method @@ -37,7 +32,7 @@ This tutorial assumes you are familiar with GitLab CI/CD and Vault. To follow along, you must have: -- An instance running GitLab 15.9 or later, or be on GitLab.com. +- An instance running GitLab 16.0 or later, or be on GitLab.com. - A Vault server that you are already using. - CI/CD jobs retrieving secrets from Vault with `CI_JOB_JWT`. diff --git a/doc/ci/secrets/id_token_authentication.md b/doc/ci/secrets/id_token_authentication.md index 697346474f8..62429a160d4 100644 --- a/doc/ci/secrets/id_token_authentication.md +++ b/doc/ci/secrets/id_token_authentication.md @@ -183,23 +183,6 @@ job_with_secrets: - access-second-db.sh --token $SECOND_DB_PASSWORD ``` - - -### Enable automatic ID token authentication (deprecated) - -WARNING: -This setting was [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/391886) in GitLab 16.0. -ID token authentication is now always available, and JSON Web Token access is always limited. - -To enable automatic ID token authentication: - -1. On the left sidebar, select **Search or go to** and find your project. -1. Select **Settings > CI/CD**. -1. Expand **Token Access**. -1. Turn on the **Limit JSON Web Token (JWT) access** toggle. - - - ## Troubleshooting ### `400: missing token` status code diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index 5f4ef2cfeac..c819037398a 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -4076,7 +4076,8 @@ job: #### `secrets:token` -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/356986) in GitLab 15.8. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/356986) in GitLab 15.8, controlled by the **Limit JSON Web Token (JWT) access** setting. +> - [Made always available and **Limit JSON Web Token (JWT) access** setting removed](https://gitlab.com/gitlab-org/gitlab/-/issues/366798) in GitLab 16.0. Use `secrets:token` to explicitly select a token to use when authenticating with Vault by referencing the token's CI/CD variable. @@ -4104,9 +4105,6 @@ job: **Additional details**: - When the `token` keyword is not set, the first ID token is used to authenticate. -- In GitLab 15.8 to 15.11, you must enable [**Limit JSON Web Token (JWT) access**](../secrets/id_token_authentication.md#enable-automatic-id-token-authentication-deprecated) for this keyword to be available. -- When **Limit JSON Web Token (JWT) access** is disabled, the `token` keyword is ignored and the `CI_JOB_JWT` - CI/CD variable is used to authenticate. ### `services` diff --git a/lib/gitlab/database/gitlab_schema.rb b/lib/gitlab/database/gitlab_schema.rb index 0bd357b7730..31ceb898eee 100644 --- a/lib/gitlab/database/gitlab_schema.rb +++ b/lib/gitlab/database/gitlab_schema.rb @@ -87,24 +87,37 @@ module Gitlab # rubocop:enable Gitlab/DocUrl end - private_class_method def self.cross_access_allowed?(type, table_schemas) + def self.cross_joins_allowed?(table_schemas, all_tables) + return true unless table_schemas.many? + table_schemas.any? do |schema| - extra_schemas = table_schemas - [schema] - extra_schemas -= Gitlab::Database.all_gitlab_schemas[schema]&.public_send(type) || [] # rubocop:disable GitlabSecurity/PublicSend - extra_schemas.empty? + schema_info = Gitlab::Database.all_gitlab_schemas[schema] + next false unless schema_info + + schema_info.allow_cross_joins?(table_schemas, all_tables) end end - def self.cross_joins_allowed?(table_schemas) - table_schemas.empty? || self.cross_access_allowed?(:allow_cross_joins, table_schemas) + def self.cross_transactions_allowed?(table_schemas, all_tables) + return true unless table_schemas.many? + + table_schemas.any? do |schema| + schema_info = Gitlab::Database.all_gitlab_schemas[schema] + next false unless schema_info + + schema_info.allow_cross_transactions?(table_schemas, all_tables) + end end - def self.cross_transactions_allowed?(table_schemas) - table_schemas.empty? || self.cross_access_allowed?(:allow_cross_transactions, table_schemas) - end + def self.cross_foreign_key_allowed?(table_schemas, all_tables) + return true if table_schemas.one? - def self.cross_foreign_key_allowed?(table_schemas) - self.cross_access_allowed?(:allow_cross_foreign_keys, table_schemas) + table_schemas.any? do |schema| + schema_info = Gitlab::Database.all_gitlab_schemas[schema] + next false unless schema_info + + schema_info.allow_cross_foreign_keys?(table_schemas, all_tables) + end end def self.dictionary_paths diff --git a/lib/gitlab/database/gitlab_schema_info.rb b/lib/gitlab/database/gitlab_schema_info.rb index 34b89cb9006..20d2b31a65c 100644 --- a/lib/gitlab/database/gitlab_schema_info.rb +++ b/lib/gitlab/database/gitlab_schema_info.rb @@ -2,6 +2,11 @@ module Gitlab module Database + GitlabSchemaInfoAllowCross = Struct.new( + :specific_tables, + keyword_init: true + ) + GitlabSchemaInfo = Struct.new( :name, :description, @@ -14,15 +19,76 @@ module Gitlab def initialize(*) super self.name = name.to_sym - self.allow_cross_joins = allow_cross_joins&.map(&:to_sym)&.freeze - self.allow_cross_transactions = allow_cross_transactions&.map(&:to_sym)&.freeze - self.allow_cross_foreign_keys = allow_cross_foreign_keys&.map(&:to_sym)&.freeze + self.allow_cross_joins = convert_array_to_hash(allow_cross_joins) + self.allow_cross_transactions = convert_array_to_hash(allow_cross_transactions) + self.allow_cross_foreign_keys = convert_array_to_hash(allow_cross_foreign_keys) end def self.load_file(yaml_file) content = YAML.load_file(yaml_file) new(**content.deep_symbolize_keys.merge(file_path: yaml_file)) end + + def allow_cross_joins?(table_schemas, all_tables) + allowed_schemas = allow_cross_joins || {} + + allowed_for?(allowed_schemas, table_schemas, all_tables) + end + + def allow_cross_transactions?(table_schemas, all_tables) + allowed_schemas = allow_cross_transactions || {} + + allowed_for?(allowed_schemas, table_schemas, all_tables) + end + + def allow_cross_foreign_keys?(table_schemas, all_tables) + allowed_schemas = allow_cross_foreign_keys || {} + + allowed_for?(allowed_schemas, table_schemas, all_tables) + end + + private + + def allowed_for?(allowed_schemas, table_schemas, all_tables) + denied_schemas = table_schemas - [name] + denied_schemas -= allowed_schemas.keys + return false unless denied_schemas.empty? + + all_tables.all? do |table| + table_schema = ::Gitlab::Database::GitlabSchema.table_schema!(table) + allowed_tables = allowed_schemas[table_schema] + + allowed_tables.nil? || allowed_tables.specific_tables.include?(table) + end + end + + # Convert from: + # - schema_a + # - schema_b: + # specific_tables: + # - table_b_of_schema_b + # - table_c_of_schema_b + # + # To: + # { :schema_a => nil, + # :schema_b => { specific_tables : [:table_b_of_schema_b, :table_c_of_schema_b] } + # } + # + def convert_array_to_hash(subject) + result = {} + + subject&.each do |item| + if item.is_a?(Hash) + item.each do |key, value| + result[key.to_sym] = GitlabSchemaInfoAllowCross.new(value || {}) + end + else + result[item.to_sym] = nil + end + end + + result.freeze + end end end end diff --git a/lib/gitlab/database/query_analyzers/prevent_cross_database_modification.rb b/lib/gitlab/database/query_analyzers/prevent_cross_database_modification.rb index a9f2b963340..fb25cb70e57 100644 --- a/lib/gitlab/database/query_analyzers/prevent_cross_database_modification.rb +++ b/lib/gitlab/database/query_analyzers/prevent_cross_database_modification.rb @@ -113,7 +113,7 @@ module Gitlab schemas = ::Gitlab::Database::GitlabSchema.table_schemas!(all_tables) schemas += ApplicationRecord.gitlab_transactions_stack - unless ::Gitlab::Database::GitlabSchema.cross_transactions_allowed?(schemas) + unless ::Gitlab::Database::GitlabSchema.cross_transactions_allowed?(schemas, all_tables) messages = [] messages << "Cross-database data modification of '#{schemas.to_a.join(", ")}' were detected within " \ "a transaction modifying the '#{all_tables.to_a.join(", ")}' tables. " diff --git a/qa/gdk/Dockerfile.gdk b/qa/gdk/Dockerfile.gdk index dbd1bd7eff4..5ffac413420 100644 --- a/qa/gdk/Dockerfile.gdk +++ b/qa/gdk/Dockerfile.gdk @@ -5,7 +5,7 @@ ENV GITLAB_LICENSE_MODE=test \ # Clone GDK at specific sha and bootstrap packages # -ARG GDK_SHA=9756ad259ec0ed356f49ed22678e2f13252b3f4f +ARG GDK_SHA=a93d0763b332efbd94856022a8877e97879869e6 RUN set -eux; \ git clone --depth 1 https://gitlab.com/gitlab-org/gitlab-development-kit.git && cd gitlab-development-kit; \ git fetch --depth 1 origin ${GDK_SHA} && git -c advice.detachedHead=false checkout ${GDK_SHA}; \ diff --git a/qa/qa/page/merge_request/index.rb b/qa/qa/page/merge_request/index.rb index ae024c16566..14623fe68a2 100644 --- a/qa/qa/page/merge_request/index.rb +++ b/qa/qa/page/merge_request/index.rb @@ -5,11 +5,11 @@ module QA module MergeRequest class Index < Page::Base view 'app/views/shared/empty_states/_merge_requests.html.haml' do - element :new_merge_request_button + element 'new-merge-request-button' end def click_new_merge_request - click_element(:new_merge_request_button) + click_element('new-merge-request-button') end end end diff --git a/qa/qa/page/merge_request/show.rb b/qa/qa/page/merge_request/show.rb index 708d582dab8..66a709c4d23 100644 --- a/qa/qa/page/merge_request/show.rb +++ b/qa/qa/page/merge_request/show.rb @@ -117,12 +117,12 @@ module QA end view 'app/views/projects/merge_requests/_description.html.haml' do - element :description_content + element 'description-content' end view 'app/views/projects/merge_requests/_mr_title.html.haml' do - element :edit_title_button - element :title_content, required: true + element 'edit-title-button' + element 'title-content', required: true end view 'app/views/projects/merge_requests/_page.html.haml' do @@ -136,8 +136,8 @@ module QA end view 'app/views/shared/_broadcast_message.html.haml' do - element :broadcast_notification_container - element :close_button + element 'broadcast-notification-container' + element 'close-button' end view 'app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue' do @@ -163,9 +163,9 @@ module QA def submit_pending_reviews # On test environments we have a broadcast message that can cover the buttons - if has_element?(:broadcast_notification_container, wait: 5) - within_element(:broadcast_notification_container) do - click_element(:close_button) + if has_element?('broadcast-notification-container', wait: 5) + within_element('broadcast-notification-container') do + click_element('close-button') end end @@ -218,7 +218,7 @@ module QA # Click by JS is needed to bypass the Moved MR actions popover # Change back to regular click_element when moved_mr_sidebar FF is removed # Rollout issue: https://gitlab.com/gitlab-org/gitlab/-/issues/385460 - click_by_javascript(find_element(:edit_title_button, skip_finished_loading_check: true)) + click_by_javascript(find_element('edit-title-button', skip_finished_loading_check: true)) end def fast_forward_not_possible? @@ -275,11 +275,11 @@ module QA end def has_title?(title) - has_element?(:title_content, text: title) + has_element?('title-content', text: title) end def has_description?(description) - has_element?(:description_content, text: description) + has_element?('description-content', text: description) end def mark_to_squash diff --git a/qa/qa/page/project/web_ide/vscode.rb b/qa/qa/page/project/web_ide/vscode.rb index 50bb880023d..cde311db3af 100644 --- a/qa/qa/page/project/web_ide/vscode.rb +++ b/qa/qa/page/project/web_ide/vscode.rb @@ -7,8 +7,8 @@ module QA module WebIDE class VSCode < Page::Base view 'app/views/shared/_broadcast_message.html.haml' do - element :broadcast_notification_container - element :close_button + element 'broadcast-notification-container' + element 'close-button' end def has_file_explorer? @@ -116,9 +116,9 @@ module QA def wait_for_ide_to_load page.driver.browser.switch_to.window(page.driver.browser.window_handles.last) # On test environments we have a broadcast message that can cover the buttons - if has_element?(:broadcast_notification_container, wait: 5) - within_element(:broadcast_notification_container) do - click_element(:close_button) + if has_element?('broadcast-notification-container', wait: 5) + within_element('broadcast-notification-container') do + click_element('close-button') end end diff --git a/spec/lib/gitlab/database/gitlab_schema_spec.rb b/spec/lib/gitlab/database/gitlab_schema_spec.rb index e402014df90..a6de695c345 100644 --- a/spec/lib/gitlab/database/gitlab_schema_spec.rb +++ b/spec/lib/gitlab/database/gitlab_schema_spec.rb @@ -226,57 +226,83 @@ RSpec.describe Gitlab::Database::GitlabSchema, feature_category: :database do allow_cross_joins: %i[gitlab_shared], allow_cross_transactions: %i[gitlab_internal gitlab_shared], allow_cross_foreign_keys: %i[] + ), + Gitlab::Database::GitlabSchemaInfo.new( + name: "gitlab_main_cell", + allow_cross_joins: [ + :gitlab_shared, + :gitlab_main, + { gitlab_main_clusterwide: { specific_tables: %w[plans] } } + ], + allow_cross_transactions: [ + :gitlab_internal, + :gitlab_shared, + :gitlab_main, + { gitlab_main_clusterwide: { specific_tables: %w[plans] } } + ], + allow_cross_foreign_keys: [ + { gitlab_main_clusterwide: { specific_tables: %w[plans] } } + ] ) ].index_by(&:name) ) end describe '.cross_joins_allowed?' do - where(:schemas, :result) do - %i[] | true - %i[gitlab_main_clusterwide gitlab_main] | true - %i[gitlab_main_clusterwide gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_internal] | false - %i[gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | true - %i[gitlab_main_clusterwide gitlab_shared] | true + where(:schemas, :tables, :result) do + %i[] | %i[] | true + %i[gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_internal] | %i[] | false + %i[gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | %i[] | true + %i[gitlab_main_clusterwide gitlab_shared] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[users namespaces] | false + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[plans namespaces] | true end with_them do - it { expect(described_class.cross_joins_allowed?(schemas)).to eq(result) } + it { expect(described_class.cross_joins_allowed?(schemas, tables)).to eq(result) } end end describe '.cross_transactions_allowed?' do - where(:schemas, :result) do - %i[] | true - %i[gitlab_main_clusterwide gitlab_main] | true - %i[gitlab_main_clusterwide gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_internal] | true - %i[gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | true - %i[gitlab_main_clusterwide gitlab_shared] | true + where(:schemas, :tables, :result) do + %i[] | %i[] | true + %i[gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_internal] | %i[] | true + %i[gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | %i[] | true + %i[gitlab_main_clusterwide gitlab_shared] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[users namespaces] | false + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[plans namespaces] | true end with_them do - it { expect(described_class.cross_transactions_allowed?(schemas)).to eq(result) } + it { expect(described_class.cross_transactions_allowed?(schemas, tables)).to eq(result) } end end describe '.cross_foreign_key_allowed?' do - where(:schemas, :result) do - %i[] | false - %i[gitlab_main_clusterwide gitlab_main] | true - %i[gitlab_main_clusterwide gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_internal] | false - %i[gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_shared] | false + where(:schemas, :tables, :result) do + %i[] | %i[] | false + %i[gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_internal] | %i[] | false + %i[gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_shared] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[users namespaces] | false + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[plans namespaces] | true end with_them do - it { expect(described_class.cross_foreign_key_allowed?(schemas)).to eq(result) } + it { expect(described_class.cross_foreign_key_allowed?(schemas, tables)).to eq(result) } end end end diff --git a/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb b/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb index 2fa4c9e562f..b807f6cd784 100644 --- a/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb +++ b/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb @@ -34,9 +34,11 @@ RSpec.describe 'cross-database foreign keys' do end def is_cross_db?(fk_record) - table_schemas = Gitlab::Database::GitlabSchema.table_schemas!([fk_record.from_table, fk_record.to_table]) + tables = [fk_record.from_table, fk_record.to_table] - !Gitlab::Database::GitlabSchema.cross_foreign_key_allowed?(table_schemas) + table_schemas = Gitlab::Database::GitlabSchema.table_schemas!(tables) + + !Gitlab::Database::GitlabSchema.cross_foreign_key_allowed?(table_schemas, tables) end it 'onlies have allowed list of cross-database foreign keys', :aggregate_failures do diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 3fc7d8f6fc8..ac07755e29c 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -69,8 +69,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to allow_value("dev.gitlab.com").for(:commit_email_hostname) } it { is_expected.not_to allow_value("@dev.gitlab").for(:commit_email_hostname) } - it { is_expected.to allow_value(true, false).for(:container_expiration_policies_enable_historic_entries) } - it { is_expected.not_to allow_value(nil).for(:container_expiration_policies_enable_historic_entries) } + it { is_expected.to validate_inclusion_of(:container_expiration_policies_enable_historic_entries).in_array([true, false]) } it { is_expected.to allow_value("myemail@gitlab.com").for(:lets_encrypt_notification_email) } it { is_expected.to allow_value(nil).for(:lets_encrypt_notification_email) } @@ -113,7 +112,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_numericality_of(:container_registry_cleanup_tags_service_max_list_size).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_data_repair_detail_worker_max_concurrency).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_expiration_policies_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to allow_value(true, false).for(:container_registry_expiration_policies_caching) } + it { is_expected.to validate_inclusion_of(:container_registry_expiration_policies_caching).in_array([true, false]) } it { is_expected.to validate_numericality_of(:container_registry_import_max_tags_count).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_import_max_retries).only_integer.is_greater_than_or_equal_to(0) } @@ -149,8 +148,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_numericality_of(:snippet_size_limit).only_integer.is_greater_than(0) } it { is_expected.to validate_numericality_of(:wiki_page_max_content_bytes).only_integer.is_greater_than_or_equal_to(1024) } - it { is_expected.to allow_value(true, false).for(:wiki_asciidoc_allow_uri_includes) } - it { is_expected.not_to allow_value(nil).for(:wiki_asciidoc_allow_uri_includes) } + it { is_expected.to validate_inclusion_of(:wiki_asciidoc_allow_uri_includes).in_array([true, false]) } it { is_expected.to validate_presence_of(:max_artifacts_size) } it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } it { is_expected.to validate_presence_of(:max_yaml_size_bytes) } @@ -162,11 +160,9 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_presence_of(:max_terraform_state_size_bytes) } it { is_expected.to validate_numericality_of(:max_terraform_state_size_bytes).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to allow_value(true, false).for(:user_defaults_to_private_profile) } - it { is_expected.not_to allow_value(nil).for(:user_defaults_to_private_profile) } + it { is_expected.to validate_inclusion_of(:user_defaults_to_private_profile).in_array([true, false]) } - it { is_expected.to allow_values([true, false]).for(:deny_all_requests_except_allowed) } - it { is_expected.not_to allow_value(nil).for(:deny_all_requests_except_allowed) } + it { is_expected.to validate_inclusion_of(:deny_all_requests_except_allowed).in_array([true, false]) } it 'ensures max_pages_size is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do is_expected.to validate_numericality_of(:max_pages_size).only_integer.is_greater_than_or_equal_to(0) @@ -254,8 +250,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to allow_value('http://example.com/').for(:public_runner_releases_url) } it { is_expected.not_to allow_value(nil).for(:public_runner_releases_url) } - it { is_expected.to allow_value([true, false]).for(:update_runner_versions_enabled) } - it { is_expected.not_to allow_value(nil).for(:update_runner_versions_enabled) } + it { is_expected.to validate_inclusion_of(:update_runner_versions_enabled).in_array([true, false]) } it { is_expected.not_to allow_value(['']).for(:valid_runner_registrars) } it { is_expected.not_to allow_value(['OBVIOUSLY_WRONG']).for(:valid_runner_registrars) } @@ -268,21 +263,17 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to allow_value(http).for(:jira_connect_proxy_url) } it { is_expected.to allow_value(https).for(:jira_connect_proxy_url) } - it { is_expected.to allow_value(true, false).for(:bulk_import_enabled) } - it { is_expected.not_to allow_value(nil).for(:bulk_import_enabled) } + it { is_expected.to validate_inclusion_of(:bulk_import_enabled).in_array([true, false]) } - it { is_expected.to allow_value(true, false).for(:allow_runner_registration_token) } - it { is_expected.not_to allow_value(nil).for(:allow_runner_registration_token) } + it { is_expected.to validate_inclusion_of(:allow_runner_registration_token).in_array([true, false]) } - it { is_expected.to allow_value(true, false).for(:gitlab_dedicated_instance) } - it { is_expected.not_to allow_value(nil).for(:gitlab_dedicated_instance) } + it { is_expected.to validate_inclusion_of(:gitlab_dedicated_instance).in_array([true, false]) } it { is_expected.not_to allow_value(apdex_slo: '10').for(:prometheus_alert_db_indicators_settings) } it { is_expected.to allow_value(nil).for(:prometheus_alert_db_indicators_settings) } it { is_expected.to allow_value(valid_prometheus_alert_db_indicators_settings).for(:prometheus_alert_db_indicators_settings) } - it { is_expected.to allow_value([true, false]).for(:silent_mode_enabled) } - it { is_expected.not_to allow_value(nil).for(:silent_mode_enabled) } + it { is_expected.to validate_inclusion_of(:silent_mode_enabled).in_array([true, false]) } it { is_expected.to allow_value(0).for(:ci_max_includes) } it { is_expected.to allow_value(200).for(:ci_max_includes) } @@ -298,16 +289,13 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.not_to allow_value(10.5).for(:ci_max_total_yaml_size_bytes) } it { is_expected.not_to allow_value(-1).for(:ci_max_total_yaml_size_bytes) } - it { is_expected.to allow_value([true, false]).for(:remember_me_enabled) } - it { is_expected.not_to allow_value(nil).for(:remember_me_enabled) } + it { is_expected.to validate_inclusion_of(:remember_me_enabled).in_array([true, false]) } it { is_expected.to validate_numericality_of(:namespace_aggregation_schedule_lease_duration_in_seconds).only_integer.is_greater_than(0) } - it { is_expected.to allow_values([true, false]).for(:instance_level_code_suggestions_enabled) } - it { is_expected.not_to allow_value(nil).for(:instance_level_code_suggestions_enabled) } + it { is_expected.to validate_inclusion_of(:instance_level_code_suggestions_enabled).in_array([true, false]) } - it { is_expected.to allow_values([true, false]).for(:package_registry_allow_anyone_to_pull_option) } - it { is_expected.not_to allow_value(nil).for(:package_registry_allow_anyone_to_pull_option) } + it { is_expected.to validate_inclusion_of(:package_registry_allow_anyone_to_pull_option).in_array([true, false]) } context 'when deactivate_dormant_users is enabled' do before do diff --git a/spec/models/container_expiration_policy_spec.rb b/spec/models/container_expiration_policy_spec.rb index e5f9fdd410e..1e911af5670 100644 --- a/spec/models/container_expiration_policy_spec.rb +++ b/spec/models/container_expiration_policy_spec.rb @@ -11,8 +11,7 @@ RSpec.describe ContainerExpirationPolicy, type: :model do it { is_expected.to validate_presence_of(:project) } describe '#enabled' do - it { is_expected.to allow_value(true, false).for(:enabled) } - it { is_expected.not_to allow_value(nil).for(:enabled) } + it { is_expected.to validate_inclusion_of(:enabled).in_array([true, false]) } end describe '#cadence' do diff --git a/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb index a58e8df45e4..203f477c1a0 100644 --- a/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb +++ b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb @@ -11,8 +11,7 @@ RSpec.describe DependencyProxy::ImageTtlGroupPolicy, type: :model do it { is_expected.to validate_presence_of(:group) } describe '#enabled' do - it { is_expected.to allow_value(true, false).for(:enabled) } - it { is_expected.not_to allow_value(nil).for(:enabled) } + it { is_expected.to validate_inclusion_of(:enabled).in_array([true, false]) } end describe '#ttl' do diff --git a/spec/models/integrations/apple_app_store_spec.rb b/spec/models/integrations/apple_app_store_spec.rb index 9864fe38d3f..ea66c382726 100644 --- a/spec/models/integrations/apple_app_store_spec.rb +++ b/spec/models/integrations/apple_app_store_spec.rb @@ -13,8 +13,7 @@ RSpec.describe Integrations::AppleAppStore, feature_category: :mobile_devops do it { is_expected.to validate_presence_of :app_store_key_id } it { is_expected.to validate_presence_of :app_store_private_key } it { is_expected.to validate_presence_of :app_store_private_key_file_name } - it { is_expected.to allow_value(true, false).for(:app_store_protected_refs) } - it { is_expected.not_to allow_value(nil).for(:app_store_protected_refs) } + it { is_expected.to validate_inclusion_of(:app_store_protected_refs).in_array([true, false]) } it { is_expected.to allow_value('aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee').for(:app_store_issuer_id) } it { is_expected.not_to allow_value('abcde').for(:app_store_issuer_id) } it { is_expected.to allow_value(File.read('spec/fixtures/ssl_key.pem')).for(:app_store_private_key) } diff --git a/spec/models/integrations/google_play_spec.rb b/spec/models/integrations/google_play_spec.rb index a0bc73378d3..c5b0c058809 100644 --- a/spec/models/integrations/google_play_spec.rb +++ b/spec/models/integrations/google_play_spec.rb @@ -20,8 +20,7 @@ RSpec.describe Integrations::GooglePlay, feature_category: :mobile_devops do it { is_expected.to allow_value('a.a.a').for(:package_name) } it { is_expected.to allow_value('com.example').for(:package_name) } it { is_expected.not_to allow_value('com').for(:package_name) } - it { is_expected.to allow_value(true, false).for(:google_play_protected_refs) } - it { is_expected.not_to allow_value(nil).for(:google_play_protected_refs) } + it { is_expected.to validate_inclusion_of(:google_play_protected_refs).in_array([true, false]) } it { is_expected.not_to allow_value('com.example.my app').for(:package_name) } it { is_expected.not_to allow_value('1com.example.myapp').for(:package_name) } it { is_expected.not_to allow_value('com.1example.myapp').for(:package_name) } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index f240670c514..1b10a3048c7 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -69,8 +69,7 @@ RSpec.describe Issue, feature_category: :team_planning do end describe 'validations' do - it { is_expected.not_to allow_value(nil).for(:confidential) } - it { is_expected.to allow_value(true, false).for(:confidential) } + it { is_expected.to validate_inclusion_of(:confidential).in_array([true, false]) } end describe 'custom validations' do diff --git a/spec/models/namespace/package_setting_spec.rb b/spec/models/namespace/package_setting_spec.rb index f3fda200fda..e6096bc9267 100644 --- a/spec/models/namespace/package_setting_spec.rb +++ b/spec/models/namespace/package_setting_spec.rb @@ -11,13 +11,9 @@ RSpec.describe Namespace::PackageSetting, feature_category: :package_registry do it { is_expected.to validate_presence_of(:namespace) } describe '#maven_duplicates_allowed' do - it { is_expected.to allow_value(true, false).for(:maven_duplicates_allowed) } - it { is_expected.not_to allow_value(nil).for(:maven_duplicates_allowed) } - it { is_expected.to allow_value(true, false).for(:generic_duplicates_allowed) } - it { is_expected.not_to allow_value(nil).for(:generic_duplicates_allowed) } - it { is_expected.to allow_value(true).for(:nuget_duplicates_allowed) } - it { is_expected.to allow_value(false).for(:nuget_duplicates_allowed) } - it { is_expected.not_to allow_value(nil).for(:nuget_duplicates_allowed) } + it { is_expected.to validate_inclusion_of(:maven_duplicates_allowed).in_array([true, false]) } + it { is_expected.to validate_inclusion_of(:generic_duplicates_allowed).in_array([true, false]) } + it { is_expected.to validate_inclusion_of(:nuget_duplicates_allowed).in_array([true, false]) } end describe 'regex values' do diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 3b890e75064..719e51018ac 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -26,8 +26,7 @@ RSpec.describe ProjectSetting, type: :model, feature_category: :groups_and_proje it { is_expected.to allow_value([]).for(:target_platforms) } it { is_expected.to validate_length_of(:issue_branch_template).is_at_most(255) } - it { is_expected.not_to allow_value(nil).for(:suggested_reviewers_enabled) } - it { is_expected.to allow_value(true, false).for(:suggested_reviewers_enabled) } + it { is_expected.to validate_inclusion_of(:suggested_reviewers_enabled).in_array([true, false]) } it 'allows any combination of the allowed target platforms' do valid_target_platform_combinations.each do |target_platforms| diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 401a85e2f82..343576de4d3 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -49,8 +49,7 @@ RSpec.describe UserPreference, feature_category: :user_profile do end describe 'pass_user_identities_to_ci_jwt' do - it { is_expected.to allow_value(true, false).for(:pass_user_identities_to_ci_jwt) } - it { is_expected.not_to allow_value(nil).for(:pass_user_identities_to_ci_jwt) } + it { is_expected.to validate_inclusion_of(:pass_user_identities_to_ci_jwt).in_array([true, false]) } it { is_expected.not_to allow_value("").for(:pass_user_identities_to_ci_jwt) } end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 7b1da1c691d..662e11f7cfb 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -1249,19 +1249,23 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do expect(json_response.length).to eq(6) end - it 'avoids N+1 queries', :aggregate_failures, :use_sql_query_cache, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/383788' do - get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } - expect(respone).to have_gitlab_http_status(:ok) + it 'avoids N+1 queries', :aggregate_failures do + get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } # warm-up + + expect(response).to have_gitlab_http_status(:ok) control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } end - create_list(:project, 2, :public, namespace: group1) + create(:project, :public, namespace: group1) + # threshold number 2 is the additional number of queries which are getting executed. + # with this we are allowing some N+1 that may already exist but is not obvious. + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132246#note_1581106553 expect do get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } - end.not_to exceed_all_query_limit(control.count) + end.to issue_same_number_of_queries_as(control).with_threshold(2) end end diff --git a/spec/support/database/prevent_cross_joins.rb b/spec/support/database/prevent_cross_joins.rb index 443216ba9df..3ff83e685ba 100644 --- a/spec/support/database/prevent_cross_joins.rb +++ b/spec/support/database/prevent_cross_joins.rb @@ -41,7 +41,7 @@ module Database schemas = ::Gitlab::Database::GitlabSchema.table_schemas!(tables) - unless ::Gitlab::Database::GitlabSchema.cross_joins_allowed?(schemas) + unless ::Gitlab::Database::GitlabSchema.cross_joins_allowed?(schemas, tables) Thread.current[:has_cross_join_exception] = true raise CrossJoinAcrossUnsupportedTablesError, "Unsupported cross-join across '#{tables.join(", ")}' querying '#{schemas.to_a.join(", ")}' discovered " \