Merge branch 'fj-42112-fix-deploy-keys-migration-mysql' into 'master'

Fix bug in security release with deploy keys migration

See merge request gitlab-org/gitlab-ce!16531

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

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

spec/migrations/populate_can_push_from_deploy_keys_projects_spec.rb

+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