diff options
48 files changed, 510 insertions, 169 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 2660e5a0e02..83697601895 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -811,6 +811,10 @@ entry. - [Propagate RemoteIP to Gitaly via Workhorse](gitlab-org/gitlab@71da945c85931bac0263c193902dc1b54e2e62da) ([merge request](gitlab-org/gitlab!103635)) - [Documentation to reflect 100MB upload limit](gitlab-org/gitlab@33063bb26ab7699802ecb2b325cc8619d6fe7b86) ([merge request](gitlab-org/gitlab!103978)) +## 15.6.3 (2022-12-21) + +No changes. + ## 15.6.2 (2022-12-05) ### Added (1 change) diff --git a/app/assets/javascripts/artifacts/graphql/queries/get_job_artifacts.query.graphql b/app/assets/javascripts/artifacts/graphql/queries/get_job_artifacts.query.graphql index 9777153999e..89a24d7891e 100644 --- a/app/assets/javascripts/artifacts/graphql/queries/get_job_artifacts.query.graphql +++ b/app/assets/javascripts/artifacts/graphql/queries/get_job_artifacts.query.graphql @@ -10,7 +10,6 @@ query getJobArtifacts( project(fullPath: $projectPath) { id jobs( - withArtifacts: true statuses: [SUCCESS, FAILED] first: $firstPageSize last: $lastPageSize diff --git a/app/assets/javascripts/locale/ensure_single_line.js b/app/assets/javascripts/locale/ensure_single_line.cjs index c2c63777001..c2c63777001 100644 --- a/app/assets/javascripts/locale/ensure_single_line.js +++ b/app/assets/javascripts/locale/ensure_single_line.cjs diff --git a/app/assets/javascripts/locale/index.js b/app/assets/javascripts/locale/index.js index ad01da2eb17..c1afabf1e35 100644 --- a/app/assets/javascripts/locale/index.js +++ b/app/assets/javascripts/locale/index.js @@ -1,5 +1,5 @@ import Jed from 'jed'; -import ensureSingleLine from './ensure_single_line'; +import ensureSingleLine from './ensure_single_line.cjs'; import sprintf from './sprintf'; const GITLAB_FALLBACK_LANGUAGE = 'en'; diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 631e697dd2f..7dc7a4e55a8 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -59,7 +59,7 @@ class Projects::IssuesController < Projects::ApplicationController push_force_frontend_feature_flag(:work_items_mvc, project&.work_items_mvc_feature_flag_enabled?) push_force_frontend_feature_flag(:work_items_mvc_2, project&.work_items_mvc_2_feature_flag_enabled?) push_frontend_feature_flag(:epic_widget_edit_confirmation, project) - push_frontend_feature_flag(:use_iid_in_work_items_path, project) + push_frontend_feature_flag(:use_iid_in_work_items_path, project&.group) push_force_frontend_feature_flag(:work_items_create_from_markdown, project&.work_items_create_from_markdown_feature_flag_enabled?) end diff --git a/app/controllers/projects/work_items_controller.rb b/app/controllers/projects/work_items_controller.rb index a118c6986f7..db9dca14aab 100644 --- a/app/controllers/projects/work_items_controller.rb +++ b/app/controllers/projects/work_items_controller.rb @@ -5,7 +5,7 @@ class Projects::WorkItemsController < Projects::ApplicationController push_force_frontend_feature_flag(:work_items, project&.work_items_feature_flag_enabled?) push_force_frontend_feature_flag(:work_items_mvc, project&.work_items_mvc_feature_flag_enabled?) push_force_frontend_feature_flag(:work_items_mvc_2, project&.work_items_mvc_2_feature_flag_enabled?) - push_frontend_feature_flag(:use_iid_in_work_items_path, project) + push_frontend_feature_flag(:use_iid_in_work_items_path, project&.group) end feature_category :team_planning diff --git a/app/graphql/types/namespace/shared_runners_setting_enum.rb b/app/graphql/types/namespace/shared_runners_setting_enum.rb index 4773e414aeb..fd067c9d803 100644 --- a/app/graphql/types/namespace/shared_runners_setting_enum.rb +++ b/app/graphql/types/namespace/shared_runners_setting_enum.rb @@ -4,10 +4,21 @@ module Types class Namespace::SharedRunnersSettingEnum < BaseEnum graphql_name 'SharedRunnersSetting' - ::Namespace::SHARED_RUNNERS_SETTINGS.each do |type| + DEPRECATED_SETTINGS = [::Namespace::SR_DISABLED_WITH_OVERRIDE].freeze + + ::Namespace::SHARED_RUNNERS_SETTINGS.excluding(DEPRECATED_SETTINGS).each do |type| value type.upcase, description: "Sharing of runners is #{type.tr('_', ' ')}.", value: type end + + value ::Namespace::SR_DISABLED_WITH_OVERRIDE.upcase, + description: "Sharing of runners is disabled and overridable.", + value: ::Namespace::SR_DISABLED_WITH_OVERRIDE, + deprecated: { + reason: :renamed, + replacement: ::Namespace::SR_DISABLED_AND_OVERRIDABLE, + milestone: "17.0" + } end end diff --git a/app/helpers/ci/runners_helper.rb b/app/helpers/ci/runners_helper.rb index 8df30ee1f0d..ac36c867baf 100644 --- a/app/helpers/ci/runners_helper.rb +++ b/app/helpers/ci/runners_helper.rb @@ -78,7 +78,7 @@ module Ci parent_shared_runners_setting: group.parent&.shared_runners_setting, runner_enabled_value: Namespace::SR_ENABLED, runner_disabled_value: Namespace::SR_DISABLED_AND_UNOVERRIDABLE, - runner_allow_override_value: Namespace::SR_DISABLED_WITH_OVERRIDE + runner_allow_override_value: Namespace::SR_DISABLED_AND_OVERRIDABLE } end diff --git a/app/models/group.rb b/app/models/group.rb index 0cdd7dd8596..96911e1024a 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -815,7 +815,7 @@ class Group < Namespace case state when SR_DISABLED_AND_UNOVERRIDABLE then disable_shared_runners! # also disallows override - when SR_DISABLED_WITH_OVERRIDE then disable_shared_runners_and_allow_override! + when SR_DISABLED_WITH_OVERRIDE, SR_DISABLED_AND_OVERRIDABLE then disable_shared_runners_and_allow_override! when SR_ENABLED then enable_shared_runners! # set both to true end end @@ -1055,7 +1055,7 @@ class Group < Namespace end def disable_shared_runners_and_allow_override! - # enabled -> disabled_with_override + # enabled -> disabled_and_overridable if shared_runners_enabled? update!( shared_runners_enabled: false, @@ -1068,7 +1068,7 @@ class Group < Namespace all_projects.update_all(shared_runners_enabled: false) - # disabled_and_unoverridable -> disabled_with_override + # disabled_and_unoverridable -> disabled_and_overridable else update!(allow_descendants_override_disabled_shared_runners: true) end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index d7d53956656..5dcda413ae5 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -33,9 +33,11 @@ class Namespace < ApplicationRecord NUMBER_OF_ANCESTORS_ALLOWED = 20 SR_DISABLED_AND_UNOVERRIDABLE = 'disabled_and_unoverridable' + # DISABLED_WITH_OVERRIDE is deprecated in favour of DISABLED_AND_OVERRIDABLE. SR_DISABLED_WITH_OVERRIDE = 'disabled_with_override' + SR_DISABLED_AND_OVERRIDABLE = 'disabled_and_overridable' SR_ENABLED = 'enabled' - SHARED_RUNNERS_SETTINGS = [SR_DISABLED_AND_UNOVERRIDABLE, SR_DISABLED_WITH_OVERRIDE, SR_ENABLED].freeze + SHARED_RUNNERS_SETTINGS = [SR_DISABLED_AND_UNOVERRIDABLE, SR_DISABLED_WITH_OVERRIDE, SR_DISABLED_AND_OVERRIDABLE, SR_ENABLED].freeze URL_MAX_LENGTH = 255 PATH_TRAILING_VIOLATIONS = %w[.git .atom .].freeze @@ -556,7 +558,7 @@ class Namespace < ApplicationRecord if shared_runners_enabled SR_ENABLED elsif allow_descendants_override_disabled_shared_runners - SR_DISABLED_WITH_OVERRIDE + SR_DISABLED_AND_OVERRIDABLE else SR_DISABLED_AND_UNOVERRIDABLE end @@ -566,10 +568,10 @@ class Namespace < ApplicationRecord case other_setting when SR_ENABLED false - when SR_DISABLED_WITH_OVERRIDE + when SR_DISABLED_WITH_OVERRIDE, SR_DISABLED_AND_OVERRIDABLE shared_runners_setting == SR_ENABLED when SR_DISABLED_AND_UNOVERRIDABLE - shared_runners_setting == SR_ENABLED || shared_runners_setting == SR_DISABLED_WITH_OVERRIDE + shared_runners_setting == SR_ENABLED || shared_runners_setting == SR_DISABLED_AND_OVERRIDABLE || shared_runners_setting == SR_DISABLED_WITH_OVERRIDE else raise ArgumentError end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 967a1e990b2..e6ccae0a22b 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -166,7 +166,7 @@ module Projects .then do |(import_url, resolved_host)| next '' if resolved_host.nil? || !import_url.scheme.in?(%w[http https]) - import_url.host.to_s + import_url.hostname.to_s end end diff --git a/app/views/admin/users/_users.html.haml b/app/views/admin/users/_users.html.haml index 96e6a264d8e..73027441fe6 100644 --- a/app/views/admin/users/_users.html.haml +++ b/app/views/admin/users/_users.html.haml @@ -48,7 +48,8 @@ .nav-controls = render_if_exists 'admin/users/admin_email_users' = render_if_exists 'admin/users/admin_export_user_permissions' - = link_to s_('AdminUsers|New user'), new_admin_user_path, class: 'btn gl-button btn-confirm btn-search float-right' + = render Pajamas::ButtonComponent.new(variant: :confirm, href: new_admin_user_path) do + = s_('AdminUsers|New user') .filtered-search-block.row-content-block.border-top-0 = form_tag admin_users_path, method: :get do diff --git a/app/views/groups/_delete_project_button.html.haml b/app/views/groups/_delete_project_button.html.haml index 54a99319418..8321e86c44f 100644 --- a/app/views/groups/_delete_project_button.html.haml +++ b/app/views/groups/_delete_project_button.html.haml @@ -1 +1,2 @@ -= link_to _('Delete'), project, data: { confirm: remove_project_message(project) }, method: :delete, class: "btn gl-button btn-danger" += render Pajamas::ButtonComponent.new(href: project, variant: :danger, method: :delete, button_options: { data: { confirm: remove_project_message(project) } }) do + = _('Delete') diff --git a/app/views/projects/_files.html.haml b/app/views/projects/_files.html.haml index 712d6fabf82..c278b15cc38 100644 --- a/app/views/projects/_files.html.haml +++ b/app/views/projects/_files.html.haml @@ -1,3 +1,4 @@ +- @no_breadcrumb_border = true - show_auto_devops_callout = show_auto_devops_callout?(@project) - is_project_overview = local_assigns.fetch(:is_project_overview, false) - ref = local_assigns.fetch(:ref) { current_ref } diff --git a/config/webpack.config.js b/config/webpack.config.js index a0c65ed4012..c76ffcb672e 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -313,21 +313,21 @@ module.exports = { use: [], }, WEBPACK_USE_ESBUILD_LOADER && { - test: /\.js$/, + test: /\.(js|cjs)$/, exclude: (modulePath) => /node_modules|vendor[\\/]assets/.test(modulePath) && !/\.vue\.js/.test(modulePath), loader: 'esbuild-loader', options: esbuildConfiguration, }, !WEBPACK_USE_ESBUILD_LOADER && { - test: /\.js$/, + test: /\.(js|cjs)$/, exclude: (modulePath) => /node_modules|vendor[\\/]assets/.test(modulePath) && !/\.vue\.js/.test(modulePath), loader: 'babel-loader', options: defaultJsOptions, }, WEBPACK_USE_ESBUILD_LOADER && { - test: /\.js$/, + test: /\.(js|cjs)$/, include: (modulePath) => /node_modules\/(monaco-worker-manager|monaco-marker-data-provider)\/index\.js/.test( modulePath, @@ -336,7 +336,7 @@ module.exports = { options: esbuildConfiguration, }, !WEBPACK_USE_ESBUILD_LOADER && { - test: /\.js$/, + test: /\.(js|cjs)$/, include: (modulePath) => /node_modules\/(monaco-worker-manager|monaco-marker-data-provider)\/index\.js/.test( modulePath, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index f71aced27ef..b95f18b417f 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -22777,8 +22777,9 @@ How to format SHA strings. | Value | Description | | ----- | ----------- | +| <a id="sharedrunnerssettingdisabled_and_overridable"></a>`DISABLED_AND_OVERRIDABLE` | Sharing of runners is disabled and overridable. | | <a id="sharedrunnerssettingdisabled_and_unoverridable"></a>`DISABLED_AND_UNOVERRIDABLE` | Sharing of runners is disabled and unoverridable. | -| <a id="sharedrunnerssettingdisabled_with_override"></a>`DISABLED_WITH_OVERRIDE` | Sharing of runners is disabled with override. | +| <a id="sharedrunnerssettingdisabled_with_override"></a>`DISABLED_WITH_OVERRIDE` **{warning-solid}** | **Deprecated** in 17.0. This was renamed. Use: `disabled_and_overridable`. | | <a id="sharedrunnerssettingenabled"></a>`ENABLED` | Sharing of runners is enabled. | ### `SnippetBlobActionEnum` diff --git a/doc/api/groups.md b/doc/api/groups.md index d017876b9c2..d547515a550 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1099,8 +1099,9 @@ The `shared_runners_setting` attribute determines whether shared runners are ena | Value | Description | |-------|-------------------------------------------------------------------------------------------------------------| | `enabled` | Enables shared runners for all projects and subgroups in this group. | -| `disabled_with_override` | Disables shared runners for all projects and subgroups in this group, but allows subgroups to override this setting. | +| `disabled_and_overridable` | Disables shared runners for all projects and subgroups in this group, but allows subgroups to override this setting. | | `disabled_and_unoverridable` | Disables shared runners for all projects and subgroups in this group, and prevents subgroups from overriding this setting. | +| `disabled_with_override` | (Deprecated. Use `disabled_and_overridable`) Disables shared runners for all projects and subgroups in this group, but allows subgroups to override this setting. | ### Upload a group avatar diff --git a/doc/architecture/blueprints/pods/index.md b/doc/architecture/blueprints/pods/index.md index 7f7725351d1..077303be30f 100644 --- a/doc/architecture/blueprints/pods/index.md +++ b/doc/architecture/blueprints/pods/index.md @@ -150,6 +150,14 @@ At this moment, GitLab.com has "social-network"-like capabilities that may not f We should evaluate if the SMB and mid market segment is interested in these features, or if not having them is acceptable in most cases. +### Self-managed + +For reasons of consistency, it is expected that self-managed instances will +adopt the pods architecture as well. To expand, self-managed instances can +continue with just a single Pod while supporting the option of adding additional +Pods. Organizations, and possible User decomposition will also be adopted for +self-managed instances. + ## High-level architecture problems to solve A number of technical issues need to be resolved to implement Pods (in no particular order). This section will be expanded. @@ -325,6 +333,7 @@ This is the list of known affected features with the proposed solutions. - [Pods: Organizations](pods-feature-organizations.md) - [Pods: Router Endpoints Classification](pods-feature-router-endpoints-classification.md) - [Pods: Schema changes (Postgres and Elasticsearch migrations)](pods-feature-schema-changes.md) +- [Pods: Backups](pods-feature-backups.md) - [Pods: Global Search](pods-feature-global-search.md) - [Pods: CI Runners](pods-feature-ci-runners.md) - [Pods: Admin Area](pods-feature-admin-area.md) diff --git a/doc/architecture/blueprints/pods/pods-feature-backups.md b/doc/architecture/blueprints/pods/pods-feature-backups.md new file mode 100644 index 00000000000..5e4de42f473 --- /dev/null +++ b/doc/architecture/blueprints/pods/pods-feature-backups.md @@ -0,0 +1,61 @@ +--- +stage: enablement +group: pods +comments: false +description: 'Pods: Backups' +--- + +This document is a work-in-progress and represents a very early state of the +Pods design. Significant aspects are not documented, though we expect to add +them in the future. This is one possible architecture for Pods, and we intend to +contrast this with alternatives before deciding which approach to implement. +This documentation will be kept even if we decide not to implement this so that +we can document the reasons for not choosing this approach. + +# Pods: Backups + +Each pods will take its own backups, and consequently have its own isolated +backup / restore procedure. + +## 1. Definition + +GitLab Backup takes a backup of the PostgreSQL database used by the application, +and also Git repository data. + +## 2. Data flow + +Each pod has a number of application databases to back up (e.g. `main`, and `ci`). + +Additionally, there may be cluster-wide metadata tables (e.g. `users` table) +which is directly accesible via PostgreSQL. + +## 3. Proposal + +### 3.1. Cluster-wide metadata + +It is currently unknown how cluster-wide metadata tables will be accessible. We +may choose to have cluster-wide metadata tables backed up separately, or have +each pod back up its copy of cluster-wide metdata tables. + +### 3.2 Consistency + +#### 3.2.1 Take backups independently + +As each pod will communicate with each other via API, and there will be no joins +to the users table, it should be acceptable for each pod to take a backup +independently of each other. + +#### 3.2.2 Enforce snapshots + +We can require that each pod take a snapshot for the PostgreSQL databases at +around the same time to allow for a consistent-enough backup. + +## 4. Evaluation + +As the number of pods increases, it will likely not be feasible to take a +snapshot at the same time for all pods. Hence taking backups independently is +the better option. + +## 4.1. Pros + +## 4.2. Cons diff --git a/doc/development/database/constraint_naming_convention.md b/doc/development/database/constraint_naming_convention.md index e9e130495e6..4ac1cd2a71d 100644 --- a/doc/development/database/constraint_naming_convention.md +++ b/doc/development/database/constraint_naming_convention.md @@ -18,7 +18,7 @@ The intent is not to retroactively change names in existing databases but rather |--------------------------|---------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------| | **Primary Key** | `pk_<table name>` | | `pk_projects` | | **Foreign Key** | `fk_<table name>_<column name>[_and_<column name>]*_<foreign table name>` | | `fk_projects_group_id_groups` | -| **Index** | `index_<table name>_on_<column name>[_and_<column name>]*[_and_<column name in partial clause>]*` | | `index_repositories_on_group_id` | +| **Index** | `index_<table name>_on_<column name>[_and_<column name>]*[_and_<column name in partial clause>]*` | Index names must be all lowercase. | `index_repositories_on_group_id` | | **Unique Constraint** | `unique_<table name>_<column name>[_and_<column name>]*` | | `unique_projects_group_id_and_name` | | **Check Constraint** | `check_<table name>_<column name>[_and_<column name>]*[_<suffix>]?` | The optional suffix should denote the type of validation, such as `length` and `enum`. It can also be used to disambiguate multiple `CHECK` constraints on the same column. | `check_projects_name_length`<br />`check_projects_type_enum`<br />`check_projects_admin1_id_and_admin2_id_differ` | | **Exclusion Constraint** | `excl_<table name>_<column name>[_and_<column name>]*_[_<suffix>]?` | The optional suffix should denote the type of exclusion being performed. | `excl_reservations_start_at_end_at_no_overlap` | diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 8f035d4aa13..bc08c19ca99 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -259,15 +259,12 @@ and therefore it does not have any records yet. ## Naming conventions -We keep column names consistent with [ActiveRecord's schema conventions](https://guides.rubyonrails.org/active_record_basics.html#schema-conventions). - -Custom index names should follow the pattern `index_#{table_name}_on_#{column_1}_and_#{column_2}_#{condition}`. +Names for database objects (such as tables, indexes, and views) must be lowercase. +Lowercase names ensure that queries with unquoted names don't cause errors. -Examples: +We keep column names consistent with [ActiveRecord's schema conventions](https://guides.rubyonrails.org/active_record_basics.html#schema-conventions). -- `index_services_on_type_and_id_and_template_when_active` -- `index_projects_on_id_service_desk_enabled` -- `index_clusters_on_enabled_cluster_type_id_and_created_at` +Custom index and constraint names should follow the [constraint naming convention guidelines](database/constraint_naming_convention.md). ### Truncate long index names diff --git a/lib/gitlab/database/reindexing/index_selection.rb b/lib/gitlab/database/reindexing/index_selection.rb index 2d384f2f9e2..f9874626105 100644 --- a/lib/gitlab/database/reindexing/index_selection.rb +++ b/lib/gitlab/database/reindexing/index_selection.rb @@ -12,6 +12,10 @@ module Gitlab # Only consider indexes beyond this size (before reindexing) INDEX_SIZE_MINIMUM = 1.gigabyte + VERY_LARGE_TABLES = %i[ + ci_builds + ].freeze + delegate :each, to: :indexes def initialize(candidates) @@ -30,13 +34,24 @@ module Gitlab # we force a N+1 pattern here and estimate bloat on a per-index # basis. - @indexes ||= candidates - .not_recently_reindexed - .where('ondisk_size_bytes >= ?', INDEX_SIZE_MINIMUM) + @indexes ||= relations_that_need_cleaning_before_deadline .sort_by(&:relative_bloat_level) # forced N+1 .reverse .select { |candidate| candidate.relative_bloat_level >= MINIMUM_RELATIVE_BLOAT } end + + def relations_that_need_cleaning_before_deadline + relation = candidates.not_recently_reindexed.where('ondisk_size_bytes >= ?', INDEX_SIZE_MINIMUM) + relation = relation.where.not(tablename: VERY_LARGE_TABLES) if too_late_for_very_large_table? + relation + end + + # The reindexing process takes place during the weekends and starting a + # reindexing action on a large table late on Sunday could span during + # Monday. We don't want this because it prevents vacuum from running. + def too_late_for_very_large_table? + Date.today.sunday? + end end end end diff --git a/lib/gitlab/import_export/version_checker.rb b/lib/gitlab/import_export/version_checker.rb index 5ec9db00d0a..ad071a4cbd7 100644 --- a/lib/gitlab/import_export/version_checker.rb +++ b/lib/gitlab/import_export/version_checker.rb @@ -34,7 +34,7 @@ module Gitlab end def different_version?(version) - Gem::Version.new(version) != Gem::Version.new(Gitlab::ImportExport.version) + Gitlab::VersionInfo.parse(version) != Gitlab::VersionInfo.parse(Gitlab::ImportExport.version) rescue StandardError => e Gitlab::Import::Logger.error( message: 'Import error', diff --git a/lib/gitlab/version_info.rb b/lib/gitlab/version_info.rb index 61de003c28d..0351c9b30b3 100644 --- a/lib/gitlab/version_info.rb +++ b/lib/gitlab/version_info.rb @@ -7,11 +7,14 @@ module Gitlab attr_reader :major, :minor, :patch VERSION_REGEX = /(\d+)\.(\d+)\.(\d+)/.freeze + # To mitigate ReDoS, limit the length of the version string we're + # willing to check + MAX_VERSION_LENGTH = 128 def self.parse(str, parse_suffix: false) if str.is_a?(self) str - elsif str && m = str.match(VERSION_REGEX) + elsif str && str.length <= MAX_VERSION_LENGTH && m = str.match(VERSION_REGEX) VersionInfo.new(m[1].to_i, m[2].to_i, m[3].to_i, parse_suffix ? m.post_match : nil) else VersionInfo.new diff --git a/qa/qa/specs/features/sanity/feature_flags_spec.rb b/qa/qa/specs/features/sanity/feature_flags_spec.rb index acb9528fe6a..36069558701 100644 --- a/qa/qa/specs/features/sanity/feature_flags_spec.rb +++ b/qa/qa/specs/features/sanity/feature_flags_spec.rb @@ -1,86 +1,88 @@ # frozen_string_literal: true module QA - RSpec.describe 'Feature flag handler sanity checks', :sanity_feature_flags do - context 'with an existing feature flag definition file' do - let(:definition) do - path = Pathname.new('../config/feature_flags') - .expand_path(Runtime::Path.qa_root) - .glob('**/*.yml') - .first - YAML.safe_load(File.read(path)) - end + RSpec.describe 'Framework sanity', :sanity_feature_flags do + describe 'Feature flag handler checks' do + context 'with an existing feature flag definition file' do + let(:definition) do + path = Pathname.new('../config/feature_flags') + .expand_path(Runtime::Path.qa_root) + .glob('**/*.yml') + .first + YAML.safe_load(File.read(path)) + end - it 'reads the correct default enabled state' do - # This test will fail if we ever remove all the feature flags, but that's very unlikely given how many there - # are and how much we rely on them. - expect(QA::Runtime::Feature.enabled?(definition['name'])).to be definition['default_enabled'] + it 'reads the correct default enabled state' do + # This test will fail if we ever remove all the feature flags, but that's very unlikely given how many there + # are and how much we rely on them. + expect(QA::Runtime::Feature.enabled?(definition['name'])).to be definition['default_enabled'] + end end - end - describe 'feature flag definition files' do - let(:file) do - path = Pathname.new("#{root}/config/feature_flags/development").expand_path(Runtime::Path.qa_root) - path.mkpath - Tempfile.new(%w[ff-test .yml], path) - end + describe 'feature flag definition files' do + let(:file) do + path = Pathname.new("#{root}/config/feature_flags/development").expand_path(Runtime::Path.qa_root) + path.mkpath + Tempfile.new(%w[ff-test .yml], path) + end - let(:flag) { Pathname.new(file.path).basename('.yml').to_s } - let(:root) { '..' } + let(:flag) { Pathname.new(file.path).basename('.yml').to_s } + let(:root) { '..' } - before do - definition = <<~YAML + before do + definition = <<~YAML name: #{flag} type: development default_enabled: #{flag_enabled} - YAML - File.write(file, definition) - end + YAML + File.write(file, definition) + end - after do - file.close! - end + after do + file.close! + end - shared_examples 'gets flag value' do - context 'with a default disabled feature flag' do - let(:flag_enabled) { 'false' } + shared_examples 'gets flag value' do + context 'with a default disabled feature flag' do + let(:flag_enabled) { 'false' } - it 'reads the flag as disabled' do - expect(QA::Runtime::Feature.enabled?(flag)).to be false - end + it 'reads the flag as disabled' do + expect(QA::Runtime::Feature.enabled?(flag)).to be false + end - it 'reads as enabled after the flag is enabled' do - QA::Runtime::Feature.enable(flag) + it 'reads as enabled after the flag is enabled' do + QA::Runtime::Feature.enable(flag) - expect { QA::Runtime::Feature.enabled?(flag) }.to eventually_be_truthy - .within(max_duration: 60, sleep_interval: 5) + expect { QA::Runtime::Feature.enabled?(flag) }.to eventually_be_truthy + .within(max_duration: 60, sleep_interval: 5) + end end - end - context 'with a default enabled feature flag' do - let(:flag_enabled) { 'true' } + context 'with a default enabled feature flag' do + let(:flag_enabled) { 'true' } - it 'reads the flag as enabled' do - expect(QA::Runtime::Feature.enabled?(flag)).to be true - end + it 'reads the flag as enabled' do + expect(QA::Runtime::Feature.enabled?(flag)).to be true + end - it 'reads as disabled after the flag is disabled' do - QA::Runtime::Feature.disable(flag) + it 'reads as disabled after the flag is disabled' do + QA::Runtime::Feature.disable(flag) - expect { QA::Runtime::Feature.enabled?(flag) }.to eventually_be_falsey - .within(max_duration: 60, sleep_interval: 5) + expect { QA::Runtime::Feature.enabled?(flag) }.to eventually_be_falsey + .within(max_duration: 60, sleep_interval: 5) + end end end - end - context 'with a CE feature flag' do - include_examples 'gets flag value' - end + context 'with a CE feature flag' do + include_examples 'gets flag value' + end - context 'with an EE feature flag' do - let(:root) { '../ee' } + context 'with an EE feature flag' do + let(:root) { '../ee' } - include_examples 'gets flag value' + include_examples 'gets flag value' + end end end end diff --git a/qa/qa/specs/features/sanity/framework_spec.rb b/qa/qa/specs/features/sanity/framework_spec.rb index fa34f525a85..5c80afe338e 100644 --- a/qa/qa/specs/features/sanity/framework_spec.rb +++ b/qa/qa/specs/features/sanity/framework_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Framework sanity checks', :orchestrated, :framework do + RSpec.describe 'Framework sanity', :orchestrated, :framework do describe 'Passing orchestrated example' do it 'succeeds' do Runtime::Browser.visit(:gitlab, Page::Main::Login) diff --git a/qa/qa/specs/features/sanity/interception_spec.rb b/qa/qa/specs/features/sanity/interception_spec.rb index f8930db3aa5..67be832055d 100644 --- a/qa/qa/specs/features/sanity/interception_spec.rb +++ b/qa/qa/specs/features/sanity/interception_spec.rb @@ -1,39 +1,41 @@ # frozen_string_literal: true module QA - RSpec.describe 'Browser request interception', :orchestrated, :framework do - before(:context) do - skip 'Only can test for chrome' unless QA::Runtime::Env.can_intercept? - end + RSpec.describe 'Framework sanity', :orchestrated, :framework do + describe 'Browser request interception' do + before(:context) do + skip 'Only can test for chrome' unless QA::Runtime::Env.can_intercept? + end - before do - Runtime::Browser.visit(:gitlab, Page::Main::Login) - end + before do + Runtime::Browser.visit(:gitlab, Page::Main::Login) + end - let(:page) { Capybara.current_session } - let(:logger) { class_double('QA::Runtime::Logger') } + let(:page) { Capybara.current_session } + let(:logger) { class_double('QA::Runtime::Logger') } - it 'intercepts failed graphql calls' do - page.execute_script <<~JS + it 'intercepts failed graphql calls' do + page.execute_script <<~JS fetch('/api/graphql', { method: 'POST', body: JSON.stringify({ query: 'query {}'}), headers: { 'Content-Type': 'application/json' } }) - JS + JS - Support::Waiter.wait_until do - !get_cached_error.nil? + Support::Waiter.wait_until do + !get_cached_error.nil? + end + expect(**get_cached_error).to include({ 'method' => 'POST', 'status' => 200, 'url' => '/api/graphql' }) end - expect(**get_cached_error).to include({ 'method' => 'POST', 'status' => 200, 'url' => '/api/graphql' }) - end - def get_cached_error - cache = page.execute_script <<~JS + def get_cached_error + cache = page.execute_script <<~JS return Interceptor.getCache() - JS + JS - cache['errors']&.first + cache['errors']&.first + end end end end diff --git a/qa/qa/specs/features/sanity/version_spec.rb b/qa/qa/specs/features/sanity/version_spec.rb index e93a8a6fea1..deefe830c36 100644 --- a/qa/qa/specs/features/sanity/version_spec.rb +++ b/qa/qa/specs/features/sanity/version_spec.rb @@ -7,31 +7,33 @@ module QA # environment variable is the version actually running. # # See https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/1179 - RSpec.describe 'Version sanity check', :smoke, only: { pipeline: [:pre, :release] } do - let(:api_client) { Runtime::API::Client.new(:gitlab) } - let(:request) { Runtime::API::Request.new(api_client, '/version') } + RSpec.describe 'Framework sanity', :smoke, only: { pipeline: [:pre, :release] } do + describe 'Version check' do + let(:api_client) { Runtime::API::Client.new(:gitlab) } + let(:request) { Runtime::API::Request.new(api_client, '/version') } - it 'is the specified version' do - # The `DEPLOY_VERSION` variable will only be provided for deploys to the - # `pre` and `release` environments, which only receive packaged releases. - # - # For these releases, `deploy_version` will be a package string (e.g., - # `13.1.3-ee.0`), and the reported version will be something like - # `13.1.3-ee`, so we only compare the leading SemVer string. - # - # | Package | Version | - # | ---------------- | -------------- | - # | 13.3.5-ee.0 | 13.3.5-ee | - # | 13.3.0-rc42.ee.0 | 13.3.0-rc42-ee | - deploy = Runtime::Env.deploy_version&.gsub(/\A(\d+\.\d+\.\d+).*\z/, '\1') + it 'is the specified version' do + # The `DEPLOY_VERSION` variable will only be provided for deploys to the + # `pre` and `release` environments, which only receive packaged releases. + # + # For these releases, `deploy_version` will be a package string (e.g., + # `13.1.3-ee.0`), and the reported version will be something like + # `13.1.3-ee`, so we only compare the leading SemVer string. + # + # | Package | Version | + # | ---------------- | -------------- | + # | 13.3.5-ee.0 | 13.3.5-ee | + # | 13.3.0-rc42.ee.0 | 13.3.0-rc42-ee | + deploy = Runtime::Env.deploy_version&.gsub(/\A(\d+\.\d+\.\d+).*\z/, '\1') - skip('No deploy version provided') if deploy.nil? || deploy.empty? + skip('No deploy version provided') if deploy.nil? || deploy.empty? - get request.url + get request.url - expect_status(200) - expect(json_body).to have_key(:version) - expect(json_body[:version]).to start_with(deploy) + expect_status(200) + expect(json_body).to have_key(:version) + expect(json_body[:version]).to start_with(deploy) + end end end end diff --git a/qa/qa/support/formatters/allure_metadata_formatter.rb b/qa/qa/support/formatters/allure_metadata_formatter.rb index c8ddbeb4536..eac74a3b961 100644 --- a/qa/qa/support/formatters/allure_metadata_formatter.rb +++ b/qa/qa/support/formatters/allure_metadata_formatter.rb @@ -3,6 +3,14 @@ module QA module Support module Formatters + # RSpec formatter to enhance metadata present in allure report + # Following additional data is added: + # * quarantine issue links + # * failure issues search link + # * ci job link + # * flaky status and test pass rate + # * devops stage and group as epic and feature behaviour tags + # class AllureMetadataFormatter < ::RSpec::Core::Formatters::BaseFormatter include Support::InfluxdbTools @@ -18,8 +26,6 @@ module QA # @param [RSpec::Core::Notifications::StartNotification] _start_notification # @return [void] def start(_start_notification) - return unless merge_request_iid # on main runs allure native history has pass rate already - save_flaky_specs log(:debug, "Fetched #{flaky_specs.length} flaky testcases!") rescue StandardError => e @@ -63,11 +69,11 @@ module QA # @param [RSpec::Core::Example] example # @return [void] def add_failure_issues_link(example) - spec_file = example.file_path.split('/').last - example.issue( - 'Failure issues', - "https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&state=opened&search=#{spec_file}" - ) + return unless example.execution_result.status == :failed + + search_query = ERB::Util.url_encode("Failure in #{example.file_path.gsub('./qa/specs/features/', '')}") + search_url = "https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&state=opened&search=#{search_query}" + example.issue('Failure issues', search_url) rescue StandardError => e log(:error, "Failed to add failure issue link for example '#{example.description}', error: #{e}") end @@ -89,10 +95,10 @@ module QA # @param [RSpec::Core::Example] example # @return [void] def set_flaky_status(example) - return unless merge_request_iid && flaky_specs.key?(example.metadata[:testcase]) + return unless flaky_specs.key?(example.metadata[:testcase]) && example.execution_result.status != :pending example.set_flaky - example.parameter("pass_rate", "#{flaky_specs[example.metadata[:testcase]].round(1)}%") + example.parameter("pass_rate", "#{flaky_specs[example.metadata[:testcase]].round(0)}%") log(:debug, "Setting spec as flaky because it's pass rate is below 98%") rescue StandardError => e log(:error, "Failed to add spec pass rate data for example '#{example.description}', error: #{e}") diff --git a/qa/spec/support/formatters/allure_metadata_formatter_spec.rb b/qa/spec/support/formatters/allure_metadata_formatter_spec.rb index d84e190fd56..ab3b753c3b0 100644 --- a/qa/spec/support/formatters/allure_metadata_formatter_spec.rb +++ b/qa/spec/support/formatters/allure_metadata_formatter_spec.rb @@ -5,25 +5,33 @@ describe QA::Support::Formatters::AllureMetadataFormatter do let(:formatter) { described_class.new(StringIO.new) } - let(:rspec_example_notification) { double('RSpec::Core::Notifications::ExampleNotification', example: rspec_example) } + let(:rspec_example_notification) do + instance_double(RSpec::Core::Notifications::ExampleNotification, example: rspec_example) + end + + # rubocop:disable RSpec/VerifiedDoubles let(:rspec_example) do double( - 'RSpec::Core::Example', + RSpec::Core::Example, tms: nil, issue: nil, add_link: nil, + set_flaky: nil, + parameter: nil, attempts: 0, - file_path: 'file/path/spec.rb', - execution_result: instance_double("RSpec::Core::Example::ExecutionResult", status: :passed), + file_path: 'spec.rb', + execution_result: instance_double(RSpec::Core::Example::ExecutionResult, status: status), metadata: { testcase: 'testcase', quarantine: { issue: 'issue' } } ) end + # rubocop:enable RSpec/VerifiedDoubles let(:ci_job) { 'ee:relative 5' } let(:ci_job_url) { 'url' } + let(:status) { :failed } before do stub_env('CI', 'true') @@ -31,16 +39,62 @@ describe QA::Support::Formatters::AllureMetadataFormatter do stub_env('CI_JOB_URL', ci_job_url) end - it "adds additional data to report" do - formatter.example_finished(rspec_example_notification) + context 'with links' do + it 'adds quarantine, failure issue and ci job links', :aggregate_failures do + formatter.example_finished(rspec_example_notification) - aggregate_failures do expect(rspec_example).to have_received(:issue).with('Quarantine issue', 'issue') expect(rspec_example).to have_received(:add_link).with(name: "Job(#{ci_job})", url: ci_job_url) expect(rspec_example).to have_received(:issue).with( 'Failure issues', - 'https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&state=opened&search=spec.rb' + 'https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&state=opened&search=Failure%20in%20spec.rb' ) end end + + context 'with flaky test data', :aggregate_failures do + let(:influx_client) { instance_double(InfluxDB2::Client, create_query_api: influx_query_api) } + let(:influx_query_api) { instance_double(InfluxDB2::QueryApi, query: data) } + let(:data) do + [ + instance_double( + InfluxDB2::FluxTable, + records: [ + instance_double(InfluxDB2::FluxRecord, values: { 'status' => 'failed', 'testcase' => 'testcase' }), + instance_double(InfluxDB2::FluxRecord, values: { 'status' => 'passed', 'testcase' => 'testcase' }) + ] + ) + ] + end + + before do + stub_env('QA_RUN_TYPE', 'package-and-test') + stub_env('QA_INFLUXDB_URL', 'url') + stub_env('QA_INFLUXDB_TOKEN', 'token') + + allow(InfluxDB2::Client).to receive(:new) { influx_client } + end + + context 'with non skipped spec' do + it 'adds flaky test data' do + formatter.start(nil) + formatter.example_finished(rspec_example_notification) + + expect(rspec_example).to have_received(:set_flaky) + expect(rspec_example).to have_received(:parameter).with('pass_rate', '50%') + end + end + + context 'with skipped spec' do + let(:status) { :pending } + + it 'skips adding flaky test data' do + formatter.start(nil) + formatter.example_finished(rspec_example_notification) + + expect(rspec_example).not_to have_received(:set_flaky) + expect(rspec_example).not_to have_received(:parameter) + end + end + end end diff --git a/scripts/frontend/extract_gettext_all.js b/scripts/frontend/extract_gettext_all.js index 0a5e2b06971..922aa85241f 100644 --- a/scripts/frontend/extract_gettext_all.js +++ b/scripts/frontend/extract_gettext_all.js @@ -6,7 +6,7 @@ const { decorateExtractorWithHelpers, } = require('gettext-extractor-vue'); const vue2TemplateCompiler = require('vue-template-compiler'); -const ensureSingleLine = require('../../app/assets/javascripts/locale/ensure_single_line'); +const ensureSingleLine = require('../../app/assets/javascripts/locale/ensure_single_line.cjs'); const args = argumentsParser .option('-f, --file <file>', 'Extract message from one single file') diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index f4d47b9ff8c..5b4839df2d3 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -74,7 +74,7 @@ FactoryBot.define do allow_descendants_override_disabled_shared_runners { false } end - trait :disabled_with_override do + trait :disabled_and_overridable do shared_runners_disabled allow_descendants_override_disabled_shared_runners end diff --git a/spec/frontend/group_settings/components/shared_runners_form_spec.js b/spec/frontend/group_settings/components/shared_runners_form_spec.js index 5282c0ed839..85475c749b0 100644 --- a/spec/frontend/group_settings/components/shared_runners_form_spec.js +++ b/spec/frontend/group_settings/components/shared_runners_form_spec.js @@ -10,7 +10,7 @@ jest.mock('~/api/groups_api'); const GROUP_ID = '99'; const RUNNER_ENABLED_VALUE = 'enabled'; const RUNNER_DISABLED_VALUE = 'disabled_and_unoverridable'; -const RUNNER_ALLOW_OVERRIDE_VALUE = 'disabled_with_override'; +const RUNNER_ALLOW_OVERRIDE_VALUE = 'disabled_and_overridable'; describe('group_settings/components/shared_runners_form', () => { let wrapper; diff --git a/spec/frontend/locale/ensure_single_line_spec.js b/spec/frontend/locale/ensure_single_line_spec.js index 20b04cab9c8..ca3d57015af 100644 --- a/spec/frontend/locale/ensure_single_line_spec.js +++ b/spec/frontend/locale/ensure_single_line_spec.js @@ -1,4 +1,4 @@ -import ensureSingleLine from '~/locale/ensure_single_line'; +import ensureSingleLine from '~/locale/ensure_single_line.cjs'; describe('locale', () => { describe('ensureSingleLine', () => { diff --git a/spec/helpers/ci/runners_helper_spec.rb b/spec/helpers/ci/runners_helper_spec.rb index 1b1edde8faf..6d14abd6574 100644 --- a/spec/helpers/ci/runners_helper_spec.rb +++ b/spec/helpers/ci/runners_helper_spec.rb @@ -103,7 +103,7 @@ RSpec.describe Ci::RunnersHelper do { runner_enabled_value: Namespace::SR_ENABLED, runner_disabled_value: Namespace::SR_DISABLED_AND_UNOVERRIDABLE, - runner_allow_override_value: Namespace::SR_DISABLED_WITH_OVERRIDE + runner_allow_override_value: Namespace::SR_DISABLED_AND_OVERRIDABLE } end @@ -197,7 +197,7 @@ RSpec.describe Ci::RunnersHelper do where(:shared_runners_setting, :is_disabled_and_unoverridable) do :shared_runners_enabled | "false" - :disabled_with_override | "false" + :disabled_and_overridable | "false" :disabled_and_unoverridable | "true" end diff --git a/spec/lib/gitlab/database/reindexing/index_selection_spec.rb b/spec/lib/gitlab/database/reindexing/index_selection_spec.rb index 2ae9037959d..9f31716ab94 100644 --- a/spec/lib/gitlab/database/reindexing/index_selection_spec.rb +++ b/spec/lib/gitlab/database/reindexing/index_selection_spec.rb @@ -74,4 +74,22 @@ RSpec.describe Gitlab::Database::Reindexing::IndexSelection do expect(subject.map(&:name).sort).to eq(not_recently_reindexed.map(&:name).sort) end end + + context 'with restricted tables' do + let!(:ci_builds) do + create( + :postgres_index_bloat_estimate, + index: create(:postgres_index, ondisk_size_bytes: 100.gigabytes, tablename: 'ci_builds'), + bloat_size_bytes: 20.gigabyte + ) + end + + context 'when executed on Saturdays', time_travel_to: '2022-12-17T09:44:07Z' do + it { expect(subject).to include(ci_builds.index) } + end + + context 'when executed on Sundays', time_travel_to: '2022-12-18T09:44:07Z' do + it { expect(subject).not_to include(ci_builds.index) } + end + end end diff --git a/spec/lib/gitlab/import_export/version_checker_spec.rb b/spec/lib/gitlab/import_export/version_checker_spec.rb index 14c62edb786..b3730d85f13 100644 --- a/spec/lib/gitlab/import_export/version_checker_spec.rb +++ b/spec/lib/gitlab/import_export/version_checker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::ImportExport::VersionChecker do +RSpec.describe Gitlab::ImportExport::VersionChecker, feature_category: :import do include ImportExport::CommonUtil let!(:shared) { Gitlab::ImportExport::Shared.new(nil) } diff --git a/spec/lib/gitlab/version_info_spec.rb b/spec/lib/gitlab/version_info_spec.rb index 078f952afad..99c7a762392 100644 --- a/spec/lib/gitlab/version_info_spec.rb +++ b/spec/lib/gitlab/version_info_spec.rb @@ -92,6 +92,8 @@ RSpec.describe Gitlab::VersionInfo do it { expect(described_class.parse("1.0.0-rc1-ee")).to eq(@v1_0_0) } it { expect(described_class.parse("git 1.0.0b1")).to eq(@v1_0_0) } it { expect(described_class.parse("git 1.0b1")).not_to be_valid } + it { expect(described_class.parse("1.1.#{'1' * described_class::MAX_VERSION_LENGTH}")).not_to be_valid } + it { expect(described_class.parse(nil)).not_to be_valid } context 'with parse_suffix: true' do let(:versions) do @@ -182,4 +184,10 @@ RSpec.describe Gitlab::VersionInfo do it { expect(@v1_0_1.without_patch).to eq(@v1_0_0) } it { expect(@v1_0_1_rc1.without_patch).to eq(@v1_0_0) } end + + describe 'MAX_VERSION_LENGTH' do + subject { described_class::MAX_VERSION_LENGTH } + + it { is_expected.to eq(128) } + end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index dfba0470d35..f243f639acf 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2648,7 +2648,81 @@ RSpec.describe Group do end end - context 'disabled_with_override' do + context 'disabled_and_overridable' do + subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_AND_OVERRIDABLE) } + + context 'top level group' do + let_it_be(:group) { create(:group, :shared_runners_disabled) } + let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } + let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } + + it 'enables allow descendants to override only for itself' do + expect { subject_and_reload(group, sub_group, project) } + .to change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and not_change { group.shared_runners_enabled } + .and not_change { sub_group.allow_descendants_override_disabled_shared_runners } + .and not_change { sub_group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } + end + end + + context 'group that its ancestors have shared Runners disabled but allows to override' do + let_it_be(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } + let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } + let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } + + it 'enables allow descendants to override' do + expect { subject_and_reload(parent, group, project) } + .to not_change { parent.allow_descendants_override_disabled_shared_runners } + .and not_change { parent.shared_runners_enabled } + .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and not_change { group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } + end + end + + context 'when parent does not allow' do + let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false) } + let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } + + it 'raises exception' do + expect { subject } + .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it') + end + + it 'does not allow descendants to override' do + expect do + begin + subject + rescue StandardError + nil + end + + parent.reload + group.reload + end.to not_change { parent.allow_descendants_override_disabled_shared_runners } + .and not_change { parent.shared_runners_enabled } + .and not_change { group.allow_descendants_override_disabled_shared_runners } + .and not_change { group.shared_runners_enabled } + end + end + + context 'top level group that has shared Runners enabled' do + let_it_be(:group) { create(:group, shared_runners_enabled: true) } + let_it_be(:sub_group) { create(:group, shared_runners_enabled: true, parent: group) } + let_it_be(:project) { create(:project, shared_runners_enabled: true, group: sub_group) } + + it 'enables allow descendants to override & disables shared runners everywhere' do + expect { subject_and_reload(group, sub_group, project) } + .to change { group.shared_runners_enabled }.from(true).to(false) + .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and change { sub_group.shared_runners_enabled }.from(true).to(false) + .and change { project.shared_runners_enabled }.from(true).to(false) + end + end + end + + context 'disabled_with_override (deprecated)' do subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_WITH_OVERRIDE) } context 'top level group' do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 80721e11049..706b73cb607 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -2114,7 +2114,7 @@ RSpec.describe Namespace do where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :shared_runners_setting) do true | true | Namespace::SR_ENABLED true | false | Namespace::SR_ENABLED - false | true | Namespace::SR_DISABLED_WITH_OVERRIDE + false | true | Namespace::SR_DISABLED_AND_OVERRIDABLE false | false | Namespace::SR_DISABLED_AND_UNOVERRIDABLE end @@ -2133,12 +2133,15 @@ RSpec.describe Namespace do where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :other_setting, :result) do true | true | Namespace::SR_ENABLED | false true | true | Namespace::SR_DISABLED_WITH_OVERRIDE | true + true | true | Namespace::SR_DISABLED_AND_OVERRIDABLE | true true | true | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | true false | true | Namespace::SR_ENABLED | false false | true | Namespace::SR_DISABLED_WITH_OVERRIDE | false + false | true | Namespace::SR_DISABLED_AND_OVERRIDABLE | false false | true | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | true false | false | Namespace::SR_ENABLED | false false | false | Namespace::SR_DISABLED_WITH_OVERRIDE | false + false | false | Namespace::SR_DISABLED_AND_OVERRIDABLE | false false | false | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | false end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b9aa5752243..0c513078783 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -6637,8 +6637,8 @@ RSpec.describe Project, factory_default: :keep do where(:shared_runners_setting, :project_shared_runners_enabled, :valid_record) do :shared_runners_enabled | true | true :shared_runners_enabled | false | true - :disabled_with_override | true | true - :disabled_with_override | false | true + :disabled_and_overridable | true | true + :disabled_and_overridable | false | true :disabled_and_unoverridable | true | false :disabled_and_unoverridable | false | true end diff --git a/spec/requests/api/graphql/mutations/groups/update_spec.rb b/spec/requests/api/graphql/mutations/groups/update_spec.rb index ea3d42a4463..a9acc593229 100644 --- a/spec/requests/api/graphql/mutations/groups/update_spec.rb +++ b/spec/requests/api/graphql/mutations/groups/update_spec.rb @@ -11,7 +11,7 @@ RSpec.describe 'GroupUpdate', feature_category: :subgroups do let(:variables) do { full_path: group.full_path, - shared_runners_setting: 'DISABLED_WITH_OVERRIDE' + shared_runners_setting: 'DISABLED_AND_OVERRIDABLE' } end @@ -52,6 +52,23 @@ RSpec.describe 'GroupUpdate', feature_category: :subgroups do expect(group.reload.shared_runners_setting).to eq(variables[:shared_runners_setting].downcase) end + context 'when using DISABLED_WITH_OVERRIDE (deprecated)' do + let(:variables) do + { + full_path: group.full_path, + shared_runners_setting: 'DISABLED_WITH_OVERRIDE' + } + end + + it 'updates shared runners settings with disabled_and_overridable' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_nil + expect(group.reload.shared_runners_setting).to eq('disabled_and_overridable') + end + end + context 'when bad arguments are provided' do let(:variables) { { full_path: '', shared_runners_setting: 'INVALID' } } diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index 3cf2c875341..75a1374a248 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -364,7 +364,7 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do let(:new_parent_group) { create(:group, shared_runners_enabled: false, allow_descendants_override_disabled_shared_runners: true) } it 'calls update service' do - expect(Groups::UpdateSharedRunnersService).to receive(:new).with(group, user, { shared_runners_setting: Namespace::SR_DISABLED_WITH_OVERRIDE }).and_call_original + expect(Groups::UpdateSharedRunnersService).to receive(:new).with(group, user, { shared_runners_setting: Namespace::SR_DISABLED_AND_OVERRIDABLE }).and_call_original transfer_service.execute(new_parent_group) end diff --git a/spec/services/groups/update_shared_runners_service_spec.rb b/spec/services/groups/update_shared_runners_service_spec.rb index 98eccedeace..a29f73a71c2 100644 --- a/spec/services/groups/update_shared_runners_service_spec.rb +++ b/spec/services/groups/update_shared_runners_service_spec.rb @@ -114,13 +114,13 @@ RSpec.describe Groups::UpdateSharedRunnersService do end context 'allow descendants to override' do - let(:params) { { shared_runners_setting: Namespace::SR_DISABLED_WITH_OVERRIDE } } + let(:params) { { shared_runners_setting: Namespace::SR_DISABLED_AND_OVERRIDABLE } } context 'top level group' do let_it_be(:group) { create(:group, :shared_runners_disabled) } it 'receives correct method and succeeds' do - expect(group).to receive(:update_shared_runners_setting!).with(Namespace::SR_DISABLED_WITH_OVERRIDE) + expect(group).to receive(:update_shared_runners_setting!).with(Namespace::SR_DISABLED_AND_OVERRIDABLE) expect(subject[:status]).to eq(:success) end @@ -135,6 +135,30 @@ RSpec.describe Groups::UpdateSharedRunnersService do expect(subject[:message]).to eq('Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it') end end + + context 'when using DISABLED_WITH_OVERRIDE (deprecated)' do + let(:params) { { shared_runners_setting: Namespace::SR_DISABLED_WITH_OVERRIDE } } + + context 'top level group' do + let_it_be(:group) { create(:group, :shared_runners_disabled) } + + it 'receives correct method and succeeds' do + expect(group).to receive(:update_shared_runners_setting!).with(Namespace::SR_DISABLED_WITH_OVERRIDE) + + expect(subject[:status]).to eq(:success) + end + end + + context 'when parent does not allow' do + let_it_be(:parent) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false) } + let_it_be(:group) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } + + it 'results error' do + expect(subject[:status]).to eq(:error) + expect(subject[:message]).to eq('Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it') + end + end + end end end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index f42ab198a04..608e9e08f87 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -995,6 +995,7 @@ RSpec.describe Projects::CreateService, '#execute' do where(:shared_runners_setting, :desired_config_for_new_project, :expected_result_for_project) do Namespace::SR_ENABLED | nil | true Namespace::SR_DISABLED_WITH_OVERRIDE | nil | false + Namespace::SR_DISABLED_AND_OVERRIDABLE | nil | false Namespace::SR_DISABLED_AND_UNOVERRIDABLE | nil | false end @@ -1017,6 +1018,8 @@ RSpec.describe Projects::CreateService, '#execute' do Namespace::SR_ENABLED | false | false Namespace::SR_DISABLED_WITH_OVERRIDE | false | false Namespace::SR_DISABLED_WITH_OVERRIDE | true | true + Namespace::SR_DISABLED_AND_OVERRIDABLE | false | false + Namespace::SR_DISABLED_AND_OVERRIDABLE | true | true Namespace::SR_DISABLED_AND_UNOVERRIDABLE | false | false end diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index bb11b2e617e..38ab7b6e2ee 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -373,6 +373,28 @@ RSpec.describe Projects::ImportService do expect(result[:status]).to eq(:success) end + + context 'when host resolves to an IPv6 address' do + before do + project.import_url = 'https://gitlab.com/gitlab-org/gitlab-development-kit' + + allow(Gitlab::UrlBlocker).to receive(:validate!) + .with(project.import_url, ports: Project::VALID_IMPORT_PORTS, schemes: Project::VALID_IMPORT_PROTOCOLS, dns_rebind_protection: true) + .and_return([Addressable::URI.parse('https://[2606:4700:90:0:f22e:fbec:5bed:a9b9]/gitlab-org/gitlab-development-kit'), 'gitlab.com']) + end + + it 'imports repository with url and additional resolved bare IPv6 address' do + expect(project.repository).to receive(:import_repository).with('https://gitlab.com/gitlab-org/gitlab-development-kit', resolved_address: '2606:4700:90:0:f22e:fbec:5bed:a9b9').and_return(true) + + expect_next_instance_of(Projects::LfsPointers::LfsImportService) do |service| + expect(service).to receive(:execute).and_return(status: :success) + end + + result = subject.execute + + expect(result[:status]).to eq(:success) + end + end end context 'when http url is provided' do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 4d75786a4c3..5171836f917 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -535,8 +535,8 @@ RSpec.describe Projects::TransferService do where(:project_shared_runners_enabled, :shared_runners_setting, :expected_shared_runners_enabled) do true | :disabled_and_unoverridable | false false | :disabled_and_unoverridable | false - true | :disabled_with_override | true - false | :disabled_with_override | false + true | :disabled_and_overridable | true + false | :disabled_and_overridable | false true | :shared_runners_enabled | true false | :shared_runners_enabled | false end diff --git a/tooling/danger/config_files.rb b/tooling/danger/config_files.rb index 436335bfc06..914605a3783 100644 --- a/tooling/danger/config_files.rb +++ b/tooling/danger/config_files.rb @@ -7,7 +7,7 @@ module Tooling module ConfigFiles SUGGEST_INTRODUCED_BY_COMMENT = <<~SUGGEST_COMMENT ```suggestion - introduced_by_url: "%<url>s" + introduced_by_url: %<url> ``` SUGGEST_COMMENT |