Migrate issue trackers data

- move sensitive data from properties to data_fields
- move title and description to service fields

changelogs/unreleased/29403-migrate-issue-tracker-data.yml

+---
+title: Migrate issue trackers data
+merge_request: 18639
+author:
+type: other

db/post_migrate/20190924152703_migrate_issue_trackers_data.rb

+# frozen_string_literal: true
+
+class MigrateIssueTrackersData < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INTERVAL = 3.minutes.to_i
+  BATCH_SIZE = 5_000
+  MIGRATION = 'MigrateIssueTrackersSensitiveData'
+
+  disable_ddl_transaction!
+
+  class Service < ActiveRecord::Base
+    self.table_name = 'services'
+    self.inheritance_column = :_type_disabled
+
+    include ::EachBatch
+  end
+
+  def up
+    relation = Service.where(category: 'issue_tracker').where("properties IS NOT NULL AND properties != '{}' AND properties != ''")
+    queue_background_migration_jobs_by_range_at_intervals(relation,
+                                                          MIGRATION,
+                                                          INTERVAL,
+                                                          batch_size: BATCH_SIZE)
+  end
+
+  def down
+    # no need
+  end
+end

lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # This migration takes all issue trackers
+    # and move data from properties to data field tables (jira_tracker_data and issue_tracker_data)
+    class MigrateIssueTrackersSensitiveData
+      delegate :select_all, :execute, :quote_string, to: :connection
+
+      # we need to define this class and set fields encryption
+      class IssueTrackerData < ApplicationRecord
+        self.table_name = 'issue_tracker_data'
+
+        def self.encryption_options
+          {
+            key: Settings.attr_encrypted_db_key_base_32,
+            encode: true,
+            mode: :per_attribute_iv,
+            algorithm: 'aes-256-gcm'
+          }
+        end
+
+        attr_encrypted :project_url, encryption_options
+        attr_encrypted :issues_url, encryption_options
+        attr_encrypted :new_issue_url, encryption_options
+      end
+
+      # we need to define this class and set fields encryption
+      class JiraTrackerData < ApplicationRecord
+        self.table_name = 'jira_tracker_data'
+
+        def self.encryption_options
+          {
+            key: Settings.attr_encrypted_db_key_base_32,
+            encode: true,
+            mode: :per_attribute_iv,
+            algorithm: 'aes-256-gcm'
+          }
+        end
+
+        attr_encrypted :url, encryption_options
+        attr_encrypted :api_url, encryption_options
+        attr_encrypted :username, encryption_options
+        attr_encrypted :password, encryption_options
+      end
+
+      def perform(start_id, stop_id)
+        columns = 'id, properties, title, description, type'
+        batch_condition = "id >= #{start_id} AND id <= #{stop_id} AND category = 'issue_tracker' \
+          AND properties IS NOT NULL AND properties != '{}' AND properties != ''"
+
+        data_subselect = "SELECT 1 \
+          FROM jira_tracker_data \
+          WHERE jira_tracker_data.service_id = services.id \
+          UNION SELECT 1 \
+          FROM issue_tracker_data \
+          WHERE issue_tracker_data.service_id = services.id"
+
+        query = "SELECT #{columns} FROM services WHERE #{batch_condition} AND NOT EXISTS (#{data_subselect})"
+
+        migrated_ids = []
+        data_to_insert(query).each do |table, data|
+          service_ids = data.map { |s| s['service_id'] }
+
+          next if service_ids.empty?
+
+          migrated_ids += service_ids
+          Gitlab::Database.bulk_insert(table, data)
+        end
+
+        return if migrated_ids.empty?
+
+        move_title_description(migrated_ids)
+      end
+
+      private
+
+      def data_to_insert(query)
+        data = { 'jira_tracker_data' => [], 'issue_tracker_data' => [] }
+        select_all(query).each do |service|
+          begin
+            properties = JSON.parse(service['properties'])
+          rescue JSON::ParserError
+            logger.warn(
+              message: 'Properties data not parsed - invalid json',
+              service_id: service['id'],
+              properties: service['properties']
+            )
+            next
+          end
+
+          if service['type'] == 'JiraService'
+            row = data_row(JiraTrackerData, jira_mapping(properties), service)
+            key = 'jira_tracker_data'
+          else
+            row = data_row(IssueTrackerData, issue_tracker_mapping(properties), service)
+            key = 'issue_tracker_data'
+          end
+
+          data[key] << row if row
+        end
+
+        data
+      end
+
+      def data_row(klass, mapping, service)
+        base_params = { service_id: service['id'], created_at: Time.current, updated_at: Time.current }
+        klass.new(mapping).slice(*klass.column_names).compact.merge(base_params)
+      end
+
+      def move_title_description(service_ids)
+        query = "UPDATE services SET \
+          title = cast(properties as json)->>'title', \
+          description = cast(properties as json)->>'description' \
+          WHERE id IN (#{service_ids.join(',')}) AND title IS NULL AND description IS NULL"
+
+        execute(query)
+      end
+
+      def jira_mapping(properties)
+        {
+          url: properties['url'],
+          api_url: properties['api_url'],
+          username: properties['username'],
+          password: properties['password']
+        }
+      end
+
+      def issue_tracker_mapping(properties)
+        {
+          project_url: properties['project_url'],
+          issues_url: properties['issues_url'],
+          new_issue_url: properties['new_issue_url']
+        }
+      end
+
+      def connection
+        @connection ||= ActiveRecord::Base.connection
+      end
+
+      def logger
+        @logger ||= Gitlab::BackgroundMigration::Logger.build
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::MigrateIssueTrackersSensitiveData, :migration, schema: 20190924152703 do
+  let(:services) { table(:services) }
+
+  # we need to define the classes due to encryption
+  class IssueTrackerData < ApplicationRecord
+    self.table_name = 'issue_tracker_data'
+
+    def self.encryption_options
+      {
+        key: Settings.attr_encrypted_db_key_base_32,
+        encode: true,
+        mode: :per_attribute_iv,
+        algorithm: 'aes-256-gcm'
+      }
+    end
+
+    attr_encrypted :project_url, encryption_options
+    attr_encrypted :issues_url, encryption_options
+    attr_encrypted :new_issue_url, encryption_options
+  end
+
+  class JiraTrackerData < ApplicationRecord
+    self.table_name = 'jira_tracker_data'
+
+    def self.encryption_options
+      {
+        key: Settings.attr_encrypted_db_key_base_32,
+        encode: true,
+        mode: :per_attribute_iv,
+        algorithm: 'aes-256-gcm'
+      }
+    end
+
+    attr_encrypted :url, encryption_options
+    attr_encrypted :api_url, encryption_options
+    attr_encrypted :username, encryption_options
+    attr_encrypted :password, encryption_options
+  end
+
+  let(:url) { 'http://base-url.tracker.com' }
+  let(:new_issue_url) { 'http://base-url.tracker.com/new_issue' }
+  let(:issues_url) { 'http://base-url.tracker.com/issues' }
+  let(:api_url) { 'http://api.tracker.com' }
+  let(:password) { 'passw1234' }
+  let(:username) { 'user9' }
+  let(:title) { 'Issue tracker' }
+  let(:description) { 'Issue tracker description' }
+
+  let(:jira_properties) do
+    {
+      'api_url' => api_url,
+      'jira_issue_transition_id' => '5',
+      'password' => password,
+      'url' => url,
+      'username' => username,
+      'title' => title,
+      'description' => description,
+      'other_field' => 'something'
+    }
+  end
+
+  let(:tracker_properties) do
+    {
+      'project_url' => url,
+      'new_issue_url' => new_issue_url,
+      'issues_url' => issues_url,
+      'title' => title,
+      'description' => description,
+      'other_field' => 'something'
+    }
+  end
+
+  let(:tracker_properties_no_url) do
+    {
+      'new_issue_url' => new_issue_url,
+      'issues_url' => issues_url,
+      'title' => title,
+      'description' => description
+    }
+  end
+
+  subject { described_class.new.perform(1, 100) }
+
+  shared_examples 'handle properties' do
+    it 'does not clear the properties' do
+      expect { subject }.not_to change { service.reload.properties}
+    end
+  end
+
+  context 'with jira service' do
+    let!(:service) do
+      services.create(id: 10, type: 'JiraService', title: nil, properties: jira_properties.to_json, category: 'issue_tracker')
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'migrates data' do
+      expect { subject }.to change { JiraTrackerData.count }.by(1)
+
+      service.reload
+      data = JiraTrackerData.find_by(service_id: service.id)
+
+      expect(data.url).to eq(url)
+      expect(data.api_url).to eq(api_url)
+      expect(data.username).to eq(username)
+      expect(data.password).to eq(password)
+      expect(service.title).to eq(title)
+      expect(service.description).to eq(description)
+    end
+  end
+
+  context 'with bugzilla service' do
+    let!(:service) do
+      services.create(id: 11, type: 'BugzillaService', title: nil, properties: tracker_properties.to_json, category: 'issue_tracker')
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'migrates data' do
+      expect { subject }.to change { IssueTrackerData.count }.by(1)
+
+      service.reload
+      data = IssueTrackerData.find_by(service_id: service.id)
+
+      expect(data.project_url).to eq(url)
+      expect(data.issues_url).to eq(issues_url)
+      expect(data.new_issue_url).to eq(new_issue_url)
+      expect(service.title).to eq(title)
+      expect(service.description).to eq(description)
+    end
+  end
+
+  context 'with youtrack service' do
+    let!(:service) do
+      services.create(id: 12, type: 'YoutrackService', title: nil, properties: tracker_properties_no_url.to_json, category: 'issue_tracker')
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'migrates data' do
+      expect { subject }.to change { IssueTrackerData.count }.by(1)
+
+      service.reload
+      data = IssueTrackerData.find_by(service_id: service.id)
+
+      expect(data.project_url).to be_nil
+      expect(data.issues_url).to eq(issues_url)
+      expect(data.new_issue_url).to eq(new_issue_url)
+      expect(service.title).to eq(title)
+      expect(service.description).to eq(description)
+    end
+  end
+
+  context 'with gitlab service with no properties' do
+    let!(:service) do
+      services.create(id: 13, type: 'GitlabIssueTrackerService', title: nil, properties: {}, category: 'issue_tracker')
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'does not migrate data' do
+      expect { subject }.not_to change { IssueTrackerData.count }
+    end
+  end
+
+  context 'with redmine service already with data fields' do
+    let!(:service) do
+      services.create(id: 14, type: 'RedmineService', title: nil, properties: tracker_properties_no_url.to_json, category: 'issue_tracker').tap do |service|
+        IssueTrackerData.create!(service_id: service.id, project_url: url, new_issue_url: new_issue_url, issues_url: issues_url)
+      end
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'does not create new data fields record' do
+      expect { subject }.not_to change { IssueTrackerData.count }
+    end
+  end
+
+  context 'with custom issue tracker which has data fields record inconsistent with properties field' do
+    let!(:service) do
+      services.create(id: 15, type: 'CustomIssueTrackerService', title: 'Existing title', properties: jira_properties.to_json, category: 'issue_tracker').tap do |service|
+        IssueTrackerData.create!(service_id: service.id, project_url: 'http://other_url', new_issue_url: 'http://other_url/new_issue', issues_url: 'http://other_url/issues')
+      end
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'does not update the data fields record' do
+      expect { subject }.not_to change { IssueTrackerData.count }
+
+      service.reload
+      data = IssueTrackerData.find_by(service_id: service.id)
+
+      expect(data.project_url).to eq('http://other_url')
+      expect(data.issues_url).to eq('http://other_url/issues')
+      expect(data.new_issue_url).to eq('http://other_url/new_issue')
+      expect(service.title).to eq('Existing title')
+    end
+  end
+
+  context 'with jira service which has data fields record inconsistent with properties field' do
+    let!(:service) do
+      services.create(id: 16, type: 'CustomIssueTrackerService', description: 'Existing description', properties: jira_properties.to_json, category: 'issue_tracker').tap do |service|
+        JiraTrackerData.create!(service_id: service.id, url: 'http://other_jira_url')
+      end
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'does not update the data fields record' do
+      expect { subject }.not_to change { JiraTrackerData.count }
+
+      service.reload
+      data = JiraTrackerData.find_by(service_id: service.id)
+
+      expect(data.url).to eq('http://other_jira_url')
+      expect(data.password).to be_nil
+      expect(data.username).to be_nil
+      expect(data.api_url).to be_nil
+      expect(service.description).to eq('Existing description')
+    end
+  end
+
+  context 'non issue tracker service' do
+    let!(:service) do
+      services.create(id: 17, title: nil, description: nil, type: 'OtherService', properties: tracker_properties.to_json)
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'does not migrate any data' do
+      expect { subject }.not_to change { IssueTrackerData.count }
+
+      service.reload
+      expect(service.title).to be_nil
+      expect(service.description).to be_nil
+    end
+  end
+
+  context 'jira service with empty properties' do
+    let!(:service) do
+      services.create(id: 18, type: 'JiraService', properties: '', category: 'issue_tracker')
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'does not migrate any data' do
+      expect { subject }.not_to change { JiraTrackerData.count }
+    end
+  end
+
+  context 'jira service with nil properties' do
+    let!(:service) do
+      services.create(id: 18, type: 'JiraService', properties: nil, category: 'issue_tracker')
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'does not migrate any data' do
+      expect { subject }.not_to change { JiraTrackerData.count }
+    end
+  end
+
+  context 'jira service with invalid properties' do
+    let!(:service) do
+      services.create(id: 18, type: 'JiraService', properties: 'invalid data', category: 'issue_tracker')
+    end
+
+    it_behaves_like 'handle properties'
+
+    it 'does not migrate any data' do
+      expect { subject }.not_to change { JiraTrackerData.count }
+    end
+  end
+
+  context 'with jira service with invalid properties, valid jira service and valid bugzilla service' do
+    let!(:jira_service_invalid) do
+      services.create(id: 19, title: 'invalid - title', description: 'invalid - description', type: 'JiraService', properties: 'invalid data', category: 'issue_tracker')
+    end
+    let!(:jira_service_valid) do
+      services.create(id: 20, type: 'JiraService', properties: jira_properties.to_json, category: 'issue_tracker')
+    end
+    let!(:bugzilla_service_valid) do
+      services.create(id: 11, type: 'BugzillaService', title: nil, properties: tracker_properties.to_json, category: 'issue_tracker')
+    end
+
+    it 'migrates data for the valid service' do
+      subject
+
+      jira_service_invalid.reload
+      expect(JiraTrackerData.find_by(service_id: jira_service_invalid.id)).to be_nil
+      expect(jira_service_invalid.title).to eq('invalid - title')
+      expect(jira_service_invalid.description).to eq('invalid - description')
+      expect(jira_service_invalid.properties).to eq('invalid data')
+
+      jira_service_valid.reload
+      data = JiraTrackerData.find_by(service_id: jira_service_valid.id)
+
+      expect(data.url).to eq(url)
+      expect(data.api_url).to eq(api_url)
+      expect(data.username).to eq(username)
+      expect(data.password).to eq(password)
+      expect(jira_service_valid.title).to eq(title)
+      expect(jira_service_valid.description).to eq(description)
+
+      bugzilla_service_valid.reload
+      data = IssueTrackerData.find_by(service_id: bugzilla_service_valid.id)
+
+      expect(data.project_url).to eq(url)
+      expect(data.issues_url).to eq(issues_url)
+      expect(data.new_issue_url).to eq(new_issue_url)
+      expect(bugzilla_service_valid.title).to eq(title)
+      expect(bugzilla_service_valid.description).to eq(description)
+    end
+  end
+end

spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20190924152703_migrate_issue_trackers_data.rb')
+
+describe MigrateIssueTrackersData, :migration do
+  let(:services) { table(:services) }
+  let(:migration_class) { Gitlab::BackgroundMigration::MigrateIssueTrackersSensitiveData }
+  let(:migration_name)  { migration_class.to_s.demodulize }
+
+  let(:properties) do
+    {
+      'url' => 'http://example.com'
+    }
+  end
+  let!(:jira_service) do
+    services.create(id: 10, type: 'JiraService', properties: properties, category: 'issue_tracker')
+  end
+  let!(:jira_service_nil) do
+    services.create(id: 11, type: 'JiraService', properties: nil, category: 'issue_tracker')
+  end
+  let!(:bugzilla_service) do
+    services.create(id: 12, type: 'BugzillaService', properties: properties, category: 'issue_tracker')
+  end
+  let!(:youtrack_service) do
+    services.create(id: 13, type: 'YoutrackService', properties: properties, category: 'issue_tracker')
+  end
+  let!(:youtrack_service_empty) do
+    services.create(id: 14, type: 'YoutrackService', properties: '', category: 'issue_tracker')
+  end
+  let!(:gitlab_service) do
+    services.create(id: 15, type: 'GitlabIssueTrackerService', properties: properties, category: 'issue_tracker')
+  end
+  let!(:gitlab_service_empty) do
+    services.create(id: 16, type: 'GitlabIssueTrackerService', properties: {}, category: 'issue_tracker')
+  end
+  let!(:other_service) do
+    services.create(id: 17, type: 'OtherService', properties: properties, category: 'other_category')
+  end
+
+  before do
+    stub_const("#{described_class}::BATCH_SIZE", 2)
+  end
+
+  it 'schedules background migrations at correct time' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(migration_name).to be_scheduled_delayed_migration(3.minutes, jira_service.id, bugzilla_service.id)
+        expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_service.id, gitlab_service.id)
+        expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+      end
+    end
+  end
+end