Add latest changes from gitlab-org/gitlab@master

.rubocop_todo/layout/first_array_element_indentation.yml

@@ -2,22 +2,6 @@
 # Cop supports --auto-correct.
 Layout/FirstArrayElementIndentation:
   Exclude:
-    - 'ee/lib/ee/api/helpers/award_emoji.rb'
-    - 'ee/spec/graphql/mutations/incident_management/escalation_policy/create_spec.rb'
-    - 'ee/spec/lib/gitlab/graphql/loaders/bulk_epic_aggregate_loader_spec.rb'
-    - 'ee/spec/models/snippet_repository_spec.rb'
-    - 'ee/spec/services/protected_environments/base_service_spec.rb'
-    - 'ee/spec/services/search_service_spec.rb'
-    - 'ee/spec/services/security/ingestion/tasks/hooks_execution_spec.rb'
-    - 'ee/spec/services/security/security_orchestration_policies/process_scan_result_policy_service_spec.rb'
-    - 'ee/spec/services/security/store_findings_metadata_service_spec.rb'
-    - 'ee/spec/services/timebox_report_service_spec.rb'
-    - 'ee/spec/services/user_permissions/export_service_spec.rb'
-    - 'ee/spec/support/shared_examples/services/search_notes_shared_examples.rb'
-    - 'ee/spec/workers/geo/scheduler/scheduler_worker_spec.rb'
-    - 'lib/event_filter.rb'
-    - 'lib/gitlab/database/migration_helpers.rb'
-    - 'lib/gitlab/email/message/in_product_marketing/team.rb'
     - 'lib/gitlab/email/message/in_product_marketing/trial.rb'
     - 'lib/gitlab/email/message/in_product_marketing/verify.rb'
     - 'lib/gitlab/import_export/base/relation_factory.rb'

GITALY_SERVER_VERSION

@@ -1 +1 @@
-c3718f5f9a3285ad8d99b4e3383edc13b1546fda
+b4c1f29c487a41b2e69a31a99f6b0ac462c81ce4

app/graphql/mutations/ci/runner/bulk_delete.rb

@@ -40,9 +40,7 @@ module Mutations
         private
 
         def model_ids_of(ids)
-          ids.map do |gid|
+          ids.filter_map { |gid| gid.model_id.to_i }
-            gid.model_id.to_i
-          end.compact
         end
 
         def find_all_runners_by_ids(ids)

app/graphql/mutations/ci/runner/update.rb

@@ -48,8 +48,13 @@ module Mutations
                  description: 'Indicates the runner is able to run untagged jobs.'
 
         argument :tag_list, [GraphQL::Types::String],
-                  required: false,
+                 required: false,
-                  description: 'Tags associated with the runner.'
+                 description: 'Tags associated with the runner.'
+
+        argument :associated_projects, [::Types::GlobalIDType[::Project]],
+                 required: false,
+                 description: 'Projects associated with the runner. Available only for project runners.',
+                 prepare: -> (global_ids, ctx) { global_ids&.filter_map { |gid| gid.model_id.to_i } }
 
         field :runner,
               Types::Ci::RunnerType,
@@ -59,15 +64,47 @@ module Mutations
         def resolve(id:, **runner_attrs)
           runner = authorized_find!(id)
 
-          result = ::Ci::Runners::UpdateRunnerService.new(runner).execute(runner_attrs)
+          associated_projects_ids = runner_attrs.delete(:associated_projects)
-          return { runner: nil, errors: result.errors } if result.error?
 
-          { runner: runner, errors: [] }
+          response = { runner: runner, errors: [] }
+          ::Ci::Runner.transaction do
+            associate_runner_projects(response, runner, associated_projects_ids) if associated_projects_ids.present?
+            update_runner(response, runner, runner_attrs)
+          end
+
+          response
         end
 
         def find_object(id)
           GitlabSchema.find_by_gid(id)
         end
+
+        private
+
+        def associate_runner_projects(response, runner, associated_project_ids)
+          unless runner.project_type?
+            raise Gitlab::Graphql::Errors::ArgumentError,
+                  "associatedProjects must not be specified for '#{runner.runner_type}' scope"
+          end
+
+          result = ::Ci::Runners::SetRunnerAssociatedProjectsService.new(
+            runner: runner,
+            current_user: current_user,
+            project_ids: associated_project_ids
+          ).execute
+          return if result.success?
+
+          response[:errors] = result.errors
+          raise ActiveRecord::Rollback
+        end
+
+        def update_runner(response, runner, attrs)
+          result = ::Ci::Runners::UpdateRunnerService.new(runner).execute(attrs)
+          return if result.success?
+
+          response[:errors] = result.errors
+          raise ActiveRecord::Rollback
+        end
       end
     end
   end

app/graphql/mutations/todos/restore_many.rb

@@ -32,9 +32,7 @@ module Mutations
       private
 
       def model_ids_of(ids)
-        ids.map do |gid|
+        ids.filter_map { |gid| gid.model_id.to_i }
-          gid.model_id.to_i
-        end.compact
       end
 
       def raise_too_many_todos_requested_error

app/services/ci/runners/set_runner_associated_projects_service.rb

@@ -17,6 +17,8 @@ module Ci
           return ServiceResponse.error(message: 'user not allowed to assign runner', http_status: :forbidden)
         end
 
+        return ServiceResponse.success if project_ids.blank?
+
         set_associated_projects
       end
 
@@ -47,16 +49,15 @@ module Ci
 
       def associate_new_projects(new_project_ids, current_project_ids)
         missing_projects = Project.id_in(new_project_ids - current_project_ids)
-        missing_projects.each do |project|
+        missing_projects.all? { |project| runner.assign_to(project, current_user) }
-          return false unless runner.assign_to(project, current_user)
-        end
-
-        true
       end
 
       def disassociate_old_projects(new_project_ids, current_project_ids)
+        projects_to_be_deleted = current_project_ids - new_project_ids
+        return true if projects_to_be_deleted.empty?
+
         Ci::RunnerProject
-          .destroy_by(project_id: current_project_ids - new_project_ids)
+          .destroy_by(project_id: projects_to_be_deleted)
           .all?(&:destroyed?)
       end
 

doc/api/graphql/reference/index.md

@@ -4495,6 +4495,7 @@ Input type: `RunnerUpdateInput`
 | ---- | ---- | ----------- |
 | <a id="mutationrunnerupdateaccesslevel"></a>`accessLevel` | [`CiRunnerAccessLevel`](#cirunneraccesslevel) | Access level of the runner. |
 | <a id="mutationrunnerupdateactive"></a>`active` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated:** This was renamed. Please use `paused`. Deprecated in 14.8. |
+| <a id="mutationrunnerupdateassociatedprojects"></a>`associatedProjects` | [`[ProjectID!]`](#projectid) | Projects associated with the runner. Available only for project runners. |
 | <a id="mutationrunnerupdateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
 | <a id="mutationrunnerupdatedescription"></a>`description` | [`String`](#string) | Description of the runner. |
 | <a id="mutationrunnerupdateid"></a>`id` | [`CiRunnerID!`](#cirunnerid) | ID of the runner to update. |

lib/event_filter.rb

@@ -131,18 +131,19 @@ class EventFilter
     finder_query = -> (id_expression) { Event.where(Event.arel_table[:id].eq(id_expression)) }
 
     if order_hint_column.present?
-      order = Gitlab::Pagination::Keyset::Order.build([
+      order = Gitlab::Pagination::Keyset::Order.build(
-        Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+        [
-          attribute_name: order_hint_column,
+          Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
-          order_expression: Event.arel_table[order_hint_column].desc,
+            attribute_name: order_hint_column,
-          nullable: :nulls_last,
+            order_expression: Event.arel_table[order_hint_column].desc,
-          distinct: false
+            nullable: :nulls_last,
-        ),
+            distinct: false
-        Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+          ),
-          attribute_name: :id,
+          Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
-          order_expression: Event.arel_table[:id].desc
+            attribute_name: :id,
-        )
+            order_expression: Event.arel_table[:id].desc
-      ])
+          )
+        ])
 
       finder_query = -> (_order_hint, id_expression) { Event.where(Event.arel_table[:id].eq(id_expression)) }
     end

lib/gitlab/database/migration_helpers.rb

@@ -936,13 +936,14 @@ module Gitlab
       def revert_backfill_conversion_of_integer_to_bigint(table, columns, primary_key: :id)
         columns = Array.wrap(columns)
 
-        conditions = ActiveRecord::Base.sanitize_sql([
+        conditions = ActiveRecord::Base.sanitize_sql(
-          'job_class_name = :job_class_name AND table_name = :table_name AND column_name = :column_name AND job_arguments = :job_arguments',
+          [
-          job_class_name: 'CopyColumnUsingBackgroundMigrationJob',
+            'job_class_name = :job_class_name AND table_name = :table_name AND column_name = :column_name AND job_arguments = :job_arguments',
-          table_name: table,
+            job_class_name: 'CopyColumnUsingBackgroundMigrationJob',
-          column_name: primary_key,
+            table_name: table,
-          job_arguments: [columns, columns.map { |column| convert_to_bigint_column(column) }].to_json
+            column_name: primary_key,
-        ])
+            job_arguments: [columns, columns.map { |column| convert_to_bigint_column(column) }].to_json
+          ])
 
         execute("DELETE FROM batched_background_migrations WHERE #{conditions}")
       end

lib/gitlab/email/message/in_product_marketing/team.rb

@@ -42,18 +42,18 @@ module Gitlab
               [
                 s_('InProductMarketing|Did you know teams that use GitLab are far more efficient?'),
                 list([
-                  s_('InProductMarketing|Goldman Sachs went from 1 build every two weeks to thousands of builds a day'),
+                       s_('InProductMarketing|Goldman Sachs went from 1 build every two weeks to thousands of builds a day'),
-                  s_('InProductMarketing|Ticketmaster decreased their CI build time by 15X')
+                       s_('InProductMarketing|Ticketmaster decreased their CI build time by 15X')
-                ])
+                     ])
               ].join("\n"),
               s_("InProductMarketing|We know a thing or two about efficiency and we don't want to keep that to ourselves. Sign up for a free trial of GitLab Ultimate and your teams will be on it from day one."),
               [
                 s_('InProductMarketing|Stop wondering and use GitLab to answer questions like:'),
                 list([
-                  s_('InProductMarketing|How long does it take us to close issues/MRs by types like feature requests, bugs, tech debt, security?'),
+                       s_('InProductMarketing|How long does it take us to close issues/MRs by types like feature requests, bugs, tech debt, security?'),
-                  s_('InProductMarketing|How many days does it take our team to complete various tasks?'),
+                       s_('InProductMarketing|How many days does it take our team to complete various tasks?'),
-                  s_('InProductMarketing|What does our value stream timeline look like from product to development to review and production?')
+                       s_('InProductMarketing|What does our value stream timeline look like from product to development to review and production?')
-                ])
+                     ])
               ].join("\n")
             ][series]
           end

qa/qa/page/component/content_editor.rb

@@ -46,8 +46,6 @@ module QA
             find_element(:file_upload_field, visible: false).send_keys(image_path)
           end
 
-          wait_for_requests
-
           QA::Support::Retrier.retry_on_exception do
             source = find_element(:wiki_hidden_content, visible: false)
             source.value =~ %r{uploads/.*#{::File.basename(image_path)}}

qa/qa/page/component/wiki_page_form.rb

@@ -35,6 +35,10 @@ module QA
         end
 
         def click_submit
+          # In case any changes were just made, wait for the hidden content field to be updated via a deferred call
+          # before clicking submit. See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97693#note_1098728562
+          sleep 0.5
+
           click_element(:wiki_submit_button)
 
           QA::Support::Retrier.retry_on_exception do

qa/qa/specs/features/browser_ui/3_create/wiki/content_editor_spec.rb

@@ -1,10 +1,7 @@
 # frozen_string_literal: true
 
 module QA
-  RSpec.describe 'Create', :reliable, quarantine: {
+  RSpec.describe 'Create', :reliable do
-    type: :investigating,
-    issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/373093'
-  } do
     context 'Content Editor' do
       let(:initial_wiki) { Resource::Wiki::ProjectPage.fabricate_via_api! }
       let(:page_title) { 'Content Editor Page' }

spec/graphql/mutations/ci/runner/update_spec.rb

@@ -6,10 +6,13 @@ RSpec.describe Mutations::Ci::Runner::Update do
   include GraphqlHelpers
 
   let_it_be(:user) { create(:user) }
-  let_it_be(:runner) { create(:ci_runner, active: true, locked: false, run_untagged: true) }
+  let_it_be(:project1) { create(:project) }
+  let_it_be(:runner) do
+    create(:ci_runner, :project, projects: [project1], active: true, locked: false, run_untagged: true)
+  end
 
   let(:current_ctx) { { current_user: user } }
-  let(:mutated_runner) { subject[:runner] }
+  let(:mutated_runner) { response[:runner] }
 
   let(:mutation_params) do
     {
@@ -21,14 +24,14 @@ RSpec.describe Mutations::Ci::Runner::Update do
   specify { expect(described_class).to require_graphql_authorizations(:update_runner) }
 
   describe '#resolve' do
-    subject do
+    subject(:response) do
       sync(resolve(described_class, args: mutation_params, ctx: current_ctx))
     end
 
     context 'when the user cannot admin the runner' do
       it 'generates an error' do
         expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do
-          subject
+          response
         end
       end
     end
@@ -37,7 +40,7 @@ RSpec.describe Mutations::Ci::Runner::Update do
       let(:mutation_params) { {} }
 
       it 'raises an error' do
-        expect { subject }.to raise_error(ArgumentError, "Arguments must be provided: id")
+        expect { response }.to raise_error(ArgumentError, "Arguments must be provided: id")
       end
     end
 
@@ -45,41 +48,150 @@ RSpec.describe Mutations::Ci::Runner::Update do
       let(:admin_user) { create(:user, :admin) }
       let(:current_ctx) { { current_user: admin_user } }
 
-      let(:mutation_params) do
-        {
-          id: runner.to_global_id,
-          description: 'updated description',
-          maintenance_note: 'updated maintenance note',
-          maximum_timeout: 900,
-          access_level: 'ref_protected',
-          active: false,
-          locked: true,
-          run_untagged: false,
-          tag_list: %w(tag1 tag2)
-        }
-      end
-
       context 'with valid arguments' do
+        let(:mutation_params) do
+          {
+            id: runner.to_global_id,
+            description: 'updated description',
+            maintenance_note: 'updated maintenance note',
+            maximum_timeout: 900,
+            access_level: 'ref_protected',
+            active: false,
+            locked: true,
+            run_untagged: false,
+            tag_list: %w(tag1 tag2)
+          }
+        end
+
         it 'updates runner with correct values' do
           expected_attributes = mutation_params.except(:id, :tag_list)
 
-          subject
+          response
 
-          expect(subject[:errors]).to be_empty
+          expect(response[:errors]).to be_empty
-          expect(subject[:runner]).to be_an_instance_of(Ci::Runner)
+          expect(response[:runner]).to be_an_instance_of(Ci::Runner)
-          expect(subject[:runner]).to have_attributes(expected_attributes)
+          expect(response[:runner]).to have_attributes(expected_attributes)
-          expect(subject[:runner].tag_list).to contain_exactly(*mutation_params[:tag_list])
+          expect(response[:runner].tag_list).to contain_exactly(*mutation_params[:tag_list])
           expect(runner.reload).to have_attributes(expected_attributes)
           expect(runner.tag_list).to contain_exactly(*mutation_params[:tag_list])
         end
       end
 
-      context 'with out-of-range maximum_timeout and missing tag_list' do
+      context 'with associatedProjects argument' do
-        it 'returns a descriptive error' do
+        let_it_be(:project2) { create(:project) }
-          mutation_params[:maximum_timeout] = 100
-          mutation_params.delete(:tag_list)
 
-          expect(subject[:errors]).to contain_exactly(
+        context 'with id set to project runner' do
+          let(:mutation_params) do
+            {
+              id: runner.to_global_id,
+              description: 'updated description',
+              associated_projects: [project2.to_global_id.to_s]
+            }
+          end
+
+          it 'updates runner attributes and project relationships', :aggregate_failures do
+            expect_next_instance_of(
+              ::Ci::Runners::SetRunnerAssociatedProjectsService,
+              {
+                runner: runner,
+                current_user: admin_user,
+                project_ids: [project2.id]
+              }
+            ) do |service|
+              expect(service).to receive(:execute).and_call_original
+            end
+
+            expected_attributes = mutation_params.except(:id, :associated_projects)
+
+            response
+
+            expect(response[:errors]).to be_empty
+            expect(response[:runner]).to be_an_instance_of(Ci::Runner)
+            expect(response[:runner]).to have_attributes(expected_attributes)
+            expect(runner.reload).to have_attributes(expected_attributes)
+            expect(runner.projects).to match_array([project1, project2])
+          end
+
+          context 'with user not allowed to assign runner' do
+            before do
+              allow(admin_user).to receive(:can?).with(:assign_runner, runner).and_return(false)
+            end
+
+            it 'does not update runner', :aggregate_failures do
+              expect_next_instance_of(
+                ::Ci::Runners::SetRunnerAssociatedProjectsService,
+                {
+                  runner: runner,
+                  current_user: admin_user,
+                  project_ids: [project2.id]
+                }
+              ) do |service|
+                expect(service).to receive(:execute).and_call_original
+              end
+
+              expected_attributes = mutation_params.except(:id, :associated_projects)
+
+              response
+
+              expect(response[:errors]).to match_array(['user not allowed to assign runner'])
+              expect(response[:runner]).to be_an_instance_of(Ci::Runner)
+              expect(response[:runner]).not_to have_attributes(expected_attributes)
+              expect(runner.reload).not_to have_attributes(expected_attributes)
+              expect(runner.projects).to match_array([project1])
+            end
+          end
+        end
+
+        context 'with id set to instance runner' do
+          let(:instance_runner) { create(:ci_runner, :instance) }
+          let(:mutation_params) do
+            {
+              id: instance_runner.to_global_id,
+              description: 'updated description',
+              associated_projects: [project2.to_global_id.to_s]
+            }
+          end
+
+          it 'raises error', :aggregate_failures do
+            expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError) do
+              response
+            end
+          end
+        end
+      end
+
+      context 'with non-existing project ID in associatedProjects argument' do
+        let(:mutation_params) do
+          {
+            id: runner.to_global_id,
+            associated_projects: ['gid://gitlab/Project/-1']
+          }
+        end
+
+        it 'does not change associated projects' do
+          expected_attributes = mutation_params.except(:id, :associated_projects)
+
+          response
+
+          expect(response[:errors]).to be_empty
+          expect(response[:runner]).to be_an_instance_of(Ci::Runner)
+          expect(response[:runner]).to have_attributes(expected_attributes)
+          expect(runner.reload).to have_attributes(expected_attributes)
+          expect(runner.projects).to match_array([project1])
+        end
+      end
+
+      context 'with out-of-range maximum_timeout and missing tag_list' do
+        let(:mutation_params) do
+          {
+            id: runner.to_global_id,
+            maximum_timeout: 100,
+            run_untagged: false
+          }
+        end
+
+        it 'returns a descriptive error' do
+          expect(response[:errors]).to contain_exactly(
             'Maximum timeout needs to be at least 10 minutes',
             'Tags list can not be empty when runner is not allowed to pick untagged jobs'
           )
@@ -90,7 +202,7 @@ RSpec.describe Mutations::Ci::Runner::Update do
         it 'returns a descriptive error' do
           mutation_params[:maintenance_note] = '1' * 1025
 
-          expect(subject[:errors]).to contain_exactly(
+          expect(response[:errors]).to contain_exactly(
             'Maintenance note is too long (maximum is 1024 characters)'
           )
         end

spec/services/ci/runners/set_runner_associated_projects_service_spec.rb

@@ -8,7 +8,8 @@ RSpec.describe ::Ci::Runners::SetRunnerAssociatedProjectsService, '#execute' do
   let_it_be(:owner_project) { create(:project) }
   let_it_be(:project2) { create(:project) }
   let_it_be(:original_projects) { [owner_project, project2] }
-  let_it_be(:runner) { create(:ci_runner, :project, projects: original_projects) }
+
+  let(:runner) { create(:ci_runner, :project, projects: original_projects) }
 
   context 'without user' do
     let(:user) { nil }
@@ -35,35 +36,52 @@ RSpec.describe ::Ci::Runners::SetRunnerAssociatedProjectsService, '#execute' do
   end
 
   context 'with admin user', :enable_admin_mode do
-    let(:user) { create_default(:user, :admin) }
+    let_it_be(:user) { create(:user, :admin) }
-    let(:project_ids) { [project3.id, project4.id] }
+
     let(:project3) { create(:project) }
     let(:project4) { create(:project) }
 
     context 'with successful requests' do
-      it 'calls assign_to on runner and returns success response' do
+      context 'when disassociating a project' do
-        expect(execute).to be_success
+        let(:project_ids) { [project3.id, project4.id] }
-        expect(runner.reload.projects.ids).to match_array([owner_project.id] + project_ids)
+
+        it 'reassigns associated projects and returns success response' do
+          expect(execute).to be_success
+          expect(runner.reload.projects.ids).to eq([owner_project.id] + project_ids)
+        end
+      end
+
+      context 'when disassociating no projects' do
+        let(:project_ids) { [project2.id, project3.id] }
+
+        it 'reassigns associated projects and returns success response' do
+          expect(execute).to be_success
+          expect(runner.reload.projects.ids).to eq([owner_project.id] + project_ids)
+        end
       end
     end
 
     context 'with failing assign_to requests' do
+      let(:project_ids) { [project3.id, project4.id] }
+
       it 'returns error response and rolls back transaction' do
         expect(runner).to receive(:assign_to).with(project4, user).once.and_return(false)
 
         expect(execute).to be_error
-        expect(runner.reload.projects).to match_array(original_projects)
+        expect(runner.reload.projects).to eq(original_projects)
       end
     end
 
     context 'with failing destroy calls' do
+      let(:project_ids) { [project3.id, project4.id] }
+
       it 'returns error response and rolls back transaction' do
         allow_next_found_instance_of(Ci::RunnerProject) do |runner_project|
           allow(runner_project).to receive(:destroy).and_return(false)
         end
 
         expect(execute).to be_error
-        expect(runner.reload.projects).to match_array(original_projects)
+        expect(runner.reload.projects).to eq(original_projects)
       end
     end
   end