From f30fa3ccd483e8acf6c1cd6e2ed25e169d0f2884 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 5 Jun 2025 18:02:48 +0000 Subject: [PATCH] Add latest changes from gitlab-org/security/gitlab@17-11-stable-ee --- .gitlab/ci/rules.gitlab-ci.yml | 5 +- ...35813_ensure_runner_taggings_exist_try2.rb | 70 +++++++++ db/schema_migrations/20250114135813 | 1 + ..._ensure_runner_taggings_exist_try2_spec.rb | 141 ++++++++++++++++++ 4 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 db/post_migrate/20250114135813_ensure_runner_taggings_exist_try2.rb create mode 100644 db/schema_migrations/20250114135813 create mode 100644 spec/migrations/20250114135813_ensure_runner_taggings_exist_try2_spec.rb diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 1fd13d7b860..1730eb71ca2 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -87,6 +87,9 @@ .if-merge-request-targeting-stable-branch: &if-merge-request-targeting-stable-branch if: '($CI_PIPELINE_SOURCE == "merge_request_event" && $CI_MERGE_REQUEST_EVENT_TYPE != "merge_train") && $CI_MERGE_REQUEST_TARGET_BRANCH_NAME =~ /^[\d-]+-stable(-ee|-jh)?$/' +.if-merge-request-targeting-stable-branch-is-tier-3: &if-merge-request-targeting-stable-branch-is-tier-3 + if: '($CI_PIPELINE_SOURCE == "merge_request_event" && $CI_MERGE_REQUEST_EVENT_TYPE != "merge_train") && $CI_MERGE_REQUEST_TARGET_BRANCH_NAME =~ /^[\d-]+-stable(-ee|-jh)?$/ && $CI_MERGE_REQUEST_LABELS =~ /pipeline::tier-3/' + .if-merge-request-labels-as-if-foss: &if-merge-request-labels-as-if-foss if: '($CI_PIPELINE_SOURCE == "merge_request_event" && $CI_MERGE_REQUEST_EVENT_TYPE != "merge_train") && $CI_MERGE_REQUEST_LABELS =~ /pipeline:run-as-if-foss/' @@ -1871,7 +1874,7 @@ - !reference [".qa:rules:e2e-test-never-run", rules] - <<: *if-merge-request-labels-run-all-e2e # this rule needs to be synced with .notify:rules:notify-test-on-omnibus-failure to produce failure notification in stable branch merge requests - - <<: *if-merge-request-targeting-stable-branch + - <<: *if-merge-request-targeting-stable-branch-is-tier-3 changes: *setup-test-env-patterns variables: MR_STABLE_BRANCH_CODE_PATTERNS: "true" diff --git a/db/post_migrate/20250114135813_ensure_runner_taggings_exist_try2.rb b/db/post_migrate/20250114135813_ensure_runner_taggings_exist_try2.rb new file mode 100644 index 00000000000..0dfeb23b242 --- /dev/null +++ b/db/post_migrate/20250114135813_ensure_runner_taggings_exist_try2.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +class EnsureRunnerTaggingsExistTry2 < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_ci + + BATCH_SIZE = 500 + + class CiRunnerMigration < MigrationRecord + include ::EachBatch + + self.primary_key = :id + end + + def up + return unless needs_migration? + + CiRunnerMigration.table_name = find_new_runners_table_name + + CiRunnerMigration.each_batch(of: BATCH_SIZE) do |batch| + scope = batch + .joins("inner join taggings on #{CiRunnerMigration.quoted_table_name}.id = taggings.taggable_id") + .where(taggings: { taggable_type: 'Ci::Runner' }) + .select(:tag_id, 'taggable_id as runner_id', :sharding_key_id, :runner_type) + + connection.execute(<<~SQL.squish) + INSERT INTO ci_runner_taggings(tag_id, runner_id, sharding_key_id, runner_type) + (#{scope.to_sql}) + ON CONFLICT DO NOTHING; + SQL + end + end + + def down; end + + private + + # Even though we introduce this at a specific timestamp, + # it could still be executed from a patch release in which the table + # was already renamed. + def find_new_runners_table_name + if connection.table_exists?(:ci_runners_e59bb2812d) + :ci_runners_e59bb2812d + else + :ci_runners + end + end + + # We don't know for sure whether the migration from taggings to + # ci_runner_taggings ran successfully. Here we take a guess and assume + # that if the ci_runner_taggings table doesn't have 50% of the entries + # from the taggings table then we need to copy the data over. This + # might have an unintended side effect of adding taggings that were + # removed, but that is generally better than the alternative of + # missing taggings altogether. + def needs_migration? + return true if ENV.key?('FORCE_COPY_RUNNER_TAGGINGS') + + old_count = disable_statement_timeout do + connection.select_value(%(SELECT COUNT(*) FROM taggings WHERE taggable_type = 'Ci::Runner')) + end + + new_count = disable_statement_timeout do + connection.select_value(%(SELECT COUNT(*) FROM ci_runner_taggings)) + end + + new_count < 0.5 * old_count + end +end diff --git a/db/schema_migrations/20250114135813 b/db/schema_migrations/20250114135813 new file mode 100644 index 00000000000..0363d15e4ab --- /dev/null +++ b/db/schema_migrations/20250114135813 @@ -0,0 +1 @@ +0d6022c79f67508f9b034d81d1ba5d3a52a3eb9a5127dd28c0fb25c4cf345e54 \ No newline at end of file diff --git a/spec/migrations/20250114135813_ensure_runner_taggings_exist_try2_spec.rb b/spec/migrations/20250114135813_ensure_runner_taggings_exist_try2_spec.rb new file mode 100644 index 00000000000..2b0222b98fa --- /dev/null +++ b/spec/migrations/20250114135813_ensure_runner_taggings_exist_try2_spec.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe EnsureRunnerTaggingsExistTry2, feature_category: :runner, migration: :gitlab_ci do + let(:runners_table) { table(:ci_runners, database: :ci, primary_key: :id) } + let(:runner_taggings_table) { table(:ci_runner_taggings, database: :ci, primary_key: :id) } + let(:taggings_table) { table(:taggings, database: :ci) } + let(:tags_table) { table(:tags, database: :ci) } + + let(:instance_runner) { runners_table.create!(runner_type: 1) } + let(:group_runner) { runners_table.create!(runner_type: 2, sharding_key_id: 10) } + let(:project_runner) { runners_table.create!(runner_type: 3, sharding_key_id: 11) } + + let(:tag1) { tags_table.create!(name: 'docker') } + let(:tag2) { tags_table.create!(name: 'postgres') } + let(:tag3) { tags_table.create!(name: 'ruby') } + let(:tag4) { tags_table.create!(name: 'golang') } + + let(:connection) { Ci::ApplicationRecord.connection } + + before do + taggings_table.create!(tag_id: tag1.id, taggable_id: instance_runner.id, + taggable_type: 'Ci::Runner', context: :tags) + taggings_table.create!(tag_id: tag2.id, taggable_id: instance_runner.id, + taggable_type: 'Ci::Runner', context: :tags) + taggings_table.create!(tag_id: tag3.id, taggable_id: instance_runner.id, + taggable_type: 'Ci::Runner', context: :tags) + taggings_table.create!(tag_id: tag1.id, taggable_id: group_runner.id, + taggable_type: 'Ci::Runner', context: :tags) + taggings_table.create!(tag_id: tag2.id, taggable_id: group_runner.id, + taggable_type: 'Ci::Runner', context: :tags) + taggings_table.create!(tag_id: tag3.id, taggable_id: project_runner.id, + taggable_type: 'Ci::Runner', context: :tags) + taggings_table.create!(tag_id: tag4.id, taggable_id: project_runner.id, + taggable_type: 'Ci::Runner', context: :tags) + + taggings_table.create!(tag_id: tag3.id, taggable_id: non_existing_record_id, + taggable_type: 'Ci::Runner', context: :tags) + taggings_table.create!(tag_id: tag3.id, taggable_id: project_runner.id, + taggable_type: 'CommitStatus', context: :tags) + end + + describe '#up' do + it 'copies records over into ci_runner_taggings' do + expect { migrate! } + .to change { runner_taggings_table.count } + .from(0) + .to(7) + + expect(tag_ids_from_taggings_for(instance_runner)) + .to match_array(runner_taggings_for(instance_runner).pluck(:tag_id)) + + expect(tag_ids_from_taggings_for(group_runner)) + .to match_array(runner_taggings_for(group_runner).pluck(:tag_id)) + + expect(tag_ids_from_taggings_for(project_runner)) + .to match_array(runner_taggings_for(project_runner).pluck(:tag_id)) + + expect(runner_taggings_for(instance_runner).pluck(:sharding_key_id).uniq) + .to contain_exactly(nil) + + expect(runner_taggings_for(group_runner).pluck(:sharding_key_id).uniq) + .to contain_exactly(10) + + expect(runner_taggings_for(project_runner).pluck(:sharding_key_id).uniq) + .to contain_exactly(11) + + expect(runner_taggings_for(non_existing_record_id)).to be_empty + end + + context 'when the table is already renamed' do + before do + connection.execute(<<~SQL.squish) + ALTER TABLE ci_runners RENAME TO _test_runners_copy; + ALTER TABLE ci_runners_e59bb2812d RENAME TO ci_runners; + SQL + end + + after do + connection.execute(<<~SQL.squish) + ALTER TABLE ci_runners RENAME TO ci_runners_e59bb2812d; + ALTER TABLE _test_runners_copy RENAME TO ci_runners; + SQL + end + + it 'copies records over into ci_runner_taggings' do + expect { migrate! } + .to change { runner_taggings_table.count } + .from(0) + .to(7) + end + end + + context 'when only 1 record exists in ci_runner_taggings' do + before do + runner_taggings_table.create!( + runner_id: instance_runner.id, + tag_id: tag1.id, + runner_type: 1 + ) + end + + it 'copies records over into ci_runner_taggings' do + expect { migrate! } + .to change { runner_taggings_table.count } + .from(1) + .to(7) + end + end + + context 'when 4 records exists in ci_runner_taggings' do + before do + [tag1, tag2, tag3, tag4].each do |tag| + runner_taggings_table.create!( + runner_id: instance_runner.id, + tag_id: tag.id, + runner_type: 1 + ) + end + end + + it 'does not copy records over into ci_runner_taggings' do + expect { migrate! } + .not_to change { runner_taggings_table.count } + .from(4) + end + end + + def tag_ids_from_taggings_for(runner) + taggings_table + .where(taggable_id: runner, taggable_type: 'Ci::Runner') + .pluck(:tag_id) + end + + def runner_taggings_for(runner) + runner_taggings_table.where(runner_id: runner) + end + end +end