From 7c9b98273bf2a63eaae37d72a4ec1b641ad425a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= Date: Wed, 17 Jan 2018 21:57:12 +0000 Subject: [10.1] Fix bug in security release with deploy keys migration --- ..._populate_can_push_from_deploy_keys_projects.rb | 48 +++++++++++++++------- ..._populate_can_push_from_deploy_keys_projects.rb | 48 +++++++++++++++------- ...late_can_push_from_deploy_keys_projects_spec.rb | 43 +++++++++++++++++++ 3 files changed, 111 insertions(+), 28 deletions(-) create mode 100644 spec/migrations/populate_can_push_from_deploy_keys_projects_spec.rb diff --git a/db/migrate/20171215113714_populate_can_push_from_deploy_keys_projects.rb b/db/migrate/20171215113714_populate_can_push_from_deploy_keys_projects.rb index ec0427a8e5a..680855af945 100644 --- a/db/migrate/20171215113714_populate_can_push_from_deploy_keys_projects.rb +++ b/db/migrate/20171215113714_populate_can_push_from_deploy_keys_projects.rb @@ -6,6 +6,8 @@ class PopulateCanPushFromDeployKeysProjects < ActiveRecord::Migration # Set this constant to true if this migration requires downtime. DOWNTIME = false + DATABASE_NAME = Gitlab::Database.database_name + disable_ddl_transaction! class DeploysKeyProject < ActiveRecord::Base @@ -18,13 +20,22 @@ class PopulateCanPushFromDeployKeysProjects < ActiveRecord::Migration DeploysKeyProject.each_batch(of: 10_000) do |batch| start_id, end_id = batch.pluck('MIN(id), MAX(id)').first - execute <<-EOF - UPDATE deploy_keys_projects - SET can_push = keys.can_push - FROM keys - WHERE deploy_key_id = keys.id - AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} - EOF + if Gitlab::Database.mysql? + execute <<-EOF.strip_heredoc + UPDATE deploy_keys_projects, #{DATABASE_NAME}.keys + SET deploy_keys_projects.can_push = #{DATABASE_NAME}.keys.can_push + WHERE deploy_keys_projects.deploy_key_id = #{DATABASE_NAME}.keys.id + AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} + EOF + else + execute <<-EOF.strip_heredoc + UPDATE deploy_keys_projects + SET can_push = keys.can_push + FROM keys + WHERE deploy_key_id = keys.id + AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} + EOF + end end end @@ -32,13 +43,22 @@ class PopulateCanPushFromDeployKeysProjects < ActiveRecord::Migration DeploysKeyProject.each_batch(of: 10_000) do |batch| start_id, end_id = batch.pluck('MIN(id), MAX(id)').first - execute <<-EOF - UPDATE keys - SET can_push = deploy_keys_projects.can_push - FROM deploy_keys_projects - WHERE deploy_keys_projects.deploy_key_id = keys.id - AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} - EOF + if Gitlab::Database.mysql? + execute <<-EOF.strip_heredoc + UPDATE deploy_keys_projects, #{DATABASE_NAME}.keys + SET #{DATABASE_NAME}.keys.can_push = deploy_keys_projects.can_push + WHERE deploy_keys_projects.deploy_key_id = #{DATABASE_NAME}.keys.id + AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} + EOF + else + execute <<-EOF.strip_heredoc + UPDATE keys + SET can_push = deploy_keys_projects.can_push + FROM deploy_keys_projects + WHERE deploy_keys_projects.deploy_key_id = keys.id + AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} + EOF + end end end end diff --git a/db/post_migrate/20171215121205_post_populate_can_push_from_deploy_keys_projects.rb b/db/post_migrate/20171215121205_post_populate_can_push_from_deploy_keys_projects.rb index 05d236f8e96..3a5850df3db 100644 --- a/db/post_migrate/20171215121205_post_populate_can_push_from_deploy_keys_projects.rb +++ b/db/post_migrate/20171215121205_post_populate_can_push_from_deploy_keys_projects.rb @@ -5,6 +5,8 @@ class PostPopulateCanPushFromDeployKeysProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers DOWNTIME = false + DATABASE_NAME = Gitlab::Database.database_name + disable_ddl_transaction! class DeploysKeyProject < ActiveRecord::Base @@ -17,13 +19,22 @@ class PostPopulateCanPushFromDeployKeysProjects < ActiveRecord::Migration DeploysKeyProject.each_batch(of: 10_000) do |batch| start_id, end_id = batch.pluck('MIN(id), MAX(id)').first - execute <<-EOF - UPDATE deploy_keys_projects - SET can_push = keys.can_push - FROM keys - WHERE deploy_key_id = keys.id - AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} - EOF + if Gitlab::Database.mysql? + execute <<-EOF.strip_heredoc + UPDATE deploy_keys_projects, #{DATABASE_NAME}.keys + SET deploy_keys_projects.can_push = #{DATABASE_NAME}.keys.can_push + WHERE deploy_keys_projects.deploy_key_id = #{DATABASE_NAME}.keys.id + AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} + EOF + else + execute <<-EOF.strip_heredoc + UPDATE deploy_keys_projects + SET can_push = keys.can_push + FROM keys + WHERE deploy_key_id = keys.id + AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} + EOF + end end end @@ -31,13 +42,22 @@ class PostPopulateCanPushFromDeployKeysProjects < ActiveRecord::Migration DeploysKeyProject.each_batch(of: 10_000) do |batch| start_id, end_id = batch.pluck('MIN(id), MAX(id)').first - execute <<-EOF - UPDATE keys - SET can_push = deploy_keys_projects.can_push - FROM deploy_keys_projects - WHERE deploy_keys_projects.deploy_key_id = keys.id - AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} - EOF + if Gitlab::Database.mysql? + execute <<-EOF.strip_heredoc + UPDATE deploy_keys_projects, #{DATABASE_NAME}.keys + SET #{DATABASE_NAME}.keys.can_push = deploy_keys_projects.can_push + WHERE deploy_keys_projects.deploy_key_id = #{DATABASE_NAME}.keys.id + AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} + EOF + else + execute <<-EOF.strip_heredoc + UPDATE keys + SET can_push = deploy_keys_projects.can_push + FROM deploy_keys_projects + WHERE deploy_keys_projects.deploy_key_id = keys.id + AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id} + EOF + end end end end diff --git a/spec/migrations/populate_can_push_from_deploy_keys_projects_spec.rb b/spec/migrations/populate_can_push_from_deploy_keys_projects_spec.rb new file mode 100644 index 00000000000..0ff98933d5c --- /dev/null +++ b/spec/migrations/populate_can_push_from_deploy_keys_projects_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20171215113714_populate_can_push_from_deploy_keys_projects.rb') + +describe PopulateCanPushFromDeployKeysProjects, :migration do + let(:migration) { described_class.new } + let(:deploy_keys) { table(:keys) } + let(:deploy_keys_projects) { table(:deploy_keys_projects) } + let(:projects) { table(:projects) } + + before do + deploy_keys.inheritance_column = nil + + projects.create!(id: 1, name: 'gitlab1', path: 'gitlab1') + (1..10).each do |index| + deploy_keys.create!(id: index, title: 'dummy', type: 'DeployKey', key: Spec::Support::Helpers::KeyGeneratorHelper.new(1024).generate + ' dummy@gitlab.com') + deploy_keys_projects.create!(id: index, deploy_key_id: index, project_id: 1) + end + end + + describe '#up' do + it 'migrates can_push from deploy_keys to deploy_keys_projects' do + deploy_keys.limit(5).update_all(can_push: true) + + expected = deploy_keys.order(:id).pluck(:id, :can_push) + + migration.up + + expect(deploy_keys_projects.order(:id).pluck(:deploy_key_id, :can_push)).to eq expected + end + end + + describe '#down' do + it 'migrates can_push from deploy_keys_projects to deploy_keys' do + deploy_keys_projects.limit(5).update_all(can_push: true) + + expected = deploy_keys_projects.order(:id).pluck(:deploy_key_id, :can_push) + + migration.down + + expect(deploy_keys.order(:id).pluck(:id, :can_push)).to eq expected + end + end +end -- cgit v1.2.1