Clean up serialized objects in audit_events

This change removes serialized ruby object in `details` hash field in
audit_events table. This causes unnecessary coupling with application
code and poses potential future breakage. This change also facilitates
future DB redesign work (e.g. jsonb migration) on this large table as it
does not require application code to be loaded.

db/post_migrate/20200518114540_schedule_fix_ruby_object_in_audit_events.rb

+# frozen_string_literal: true
+
+class ScheduleFixRubyObjectInAuditEvents < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_audit_events_on_ruby_object_in_details'
+  INTERVAL = 2.minutes.to_i
+  BATCH_SIZE = 1_000
+  MIGRATION = 'FixRubyObjectInAuditEvents'
+
+  disable_ddl_transaction!
+
+  class AuditEvent < ActiveRecord::Base
+    self.table_name = 'audit_events'
+
+    include ::EachBatch
+  end
+
+  def up
+    return unless Gitlab.ee?
+
+    # create temporary index for audit_events with ruby/object in details field, may take well over 1h
+    add_concurrent_index(:audit_events, :id, where: "details ~~ '%ruby/object%'", name: INDEX_NAME)
+
+    relation = AuditEvent.where("details ~~ '%ruby/object%'")
+
+    queue_background_migration_jobs_by_range_at_intervals(
+      relation,
+      MIGRATION,
+      INTERVAL,
+      batch_size: BATCH_SIZE
+    )
+  end
+
+  def down
+    # temporary index is to be dropped in a different migration in an upcoming release
+    # https://gitlab.com/gitlab-org/gitlab/issues/196842
+    remove_concurrent_index_by_name(:audit_events, INDEX_NAME)
+  end
+end

db/structure.sql

@@ -9229,6 +9229,8 @@ CREATE INDEX index_approvers_on_user_id ON public.approvers USING btree (user_id
 
 CREATE INDEX index_audit_events_on_entity_id_and_entity_type_and_id_desc ON public.audit_events USING btree (entity_id, entity_type, id DESC);
 
+CREATE INDEX index_audit_events_on_ruby_object_in_details ON public.audit_events USING btree (id) WHERE (details ~~ '%ruby/object%'::text);
+
 CREATE INDEX index_award_emoji_on_awardable_type_and_awardable_id ON public.award_emoji USING btree (awardable_type, awardable_id);
 
 CREATE INDEX index_award_emoji_on_user_id_and_name ON public.award_emoji USING btree (user_id, name);
@@ -13764,6 +13766,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200514000340
 20200515155620
 20200518091745
+20200518114540
 20200518133123
 20200519074709
 20200519101002

ee/changelogs/unreleased/216577-fix-ruby-object-audit-events-migrations.yml

+---
+title: Clean up serialized objects in audit_events
+merge_request: 32434
+author:
+type: fixed

ee/lib/ee/gitlab/background_migration/fix_ruby_object_in_audit_events.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module BackgroundMigration
+      module FixRubyObjectInAuditEvents
+        extend ::Gitlab::Utils::Override
+
+        override :perform
+        def perform(start_id, stop_id)
+          ActiveRecord::Base.connection.execute <<~SQL
+            UPDATE
+              audit_events
+            SET
+              details = regexp_replace(details, '!ruby/object.*name: ', '')
+            WHERE id BETWEEN #{Integer(start_id)} AND #{Integer(stop_id)}
+              AND details ~~ '%ruby/object%'
+          SQL
+        end
+      end
+    end
+  end
+end

ee/spec/lib/ee/gitlab/background_migration/fix_ruby_object_in_audit_events_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::FixRubyObjectInAuditEvents, :migration, schema: 20200518114540 do
+  let(:audit_events) { table(:audit_events) }
+
+  it 'cleans up ruby/object in details field', :aggregate_failures do
+    tainted_audit_event = audit_events.create!(
+      author_id: -1,
+      type: 'SecurityEvent',
+      entity_id: 1,
+      entity_type: 'User',
+      details: "---\n:failed_login: STANDARD\n:author_name: hacker\n" \
+               ":target_details: !ruby/object:Gitlab::Audit::UnauthenticatedAuthor\n  id: -1\n  name: hacker\n" \
+               ":ip_address: \n:entity_path: \n"
+    )
+
+    clean_audit_event = audit_events.create!(
+      author_id: 1,
+      type: 'SecurityEvent',
+      entity_id: 1,
+      entity_type: 'User',
+      details: "---\n:failed_login: STANDARD\n:author_name: homer\n" \
+               ":target_details: homer\n" \
+               ":ip_address: \n:entity_path: \n"
+    )
+
+    described_class.new.perform(tainted_audit_event.id, clean_audit_event.id)
+
+    expect(tainted_audit_event.reload.details).to eq(
+      "---\n:failed_login: STANDARD\n:author_name: hacker\n" \
+      ":target_details: hacker\n" \
+      ":ip_address: \n:entity_path: \n"
+    )
+
+    expect(clean_audit_event.reload.details).to eq(
+      "---\n:failed_login: STANDARD\n:author_name: homer\n" \
+      ":target_details: homer\n" \
+      ":ip_address: \n:entity_path: \n"
+    )
+  end
+end

ee/spec/migrations/schedule_fix_ruby_object_in_audit_events_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200518114540_schedule_fix_ruby_object_in_audit_events.rb')
+
+describe ScheduleFixRubyObjectInAuditEvents do
+  let(:audit_events) { table(:audit_events) }
+
+  it 'schedules background migrations' do
+    stub_const("#{described_class.name}::BATCH_SIZE", 1)
+
+    audit_events.create!(
+      author_id: -1,
+      type: 'SecurityEvent',
+      entity_id: 1,
+      entity_type: 'User',
+      details: "---\n:failed_login: STANDARD\n:author_name: hacker\n" \
+               ":target_details: !ruby/object:Gitlab::Audit::UnauthenticatedAuthor\n  id: -1\n  name: hacker\n" \
+               ":ip_address: \n:entity_path: \n"
+    )
+
+    audit_events.create!(
+      author_id: 1,
+      type: 'SecurityEvent',
+      entity_id: 1,
+      entity_type: 'User',
+      details: "---\n:failed_login: STANDARD\n:author_name: homer\n" \
+               ":target_details: homer\n" \
+               ":ip_address: \n:entity_path: \n"
+    )
+
+    Sidekiq::Testing.fake! do
+      migrate!
+
+      expect(BackgroundMigrationWorker.jobs.size).to eq(1)
+    end
+  end
+end

lib/gitlab/background_migration/fix_ruby_object_in_audit_events.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Remove serialized Ruby object in audit_events
+    class FixRubyObjectInAuditEvents
+      def perform(start_id, stop_id)
+      end
+    end
+  end
+end
+
+Gitlab::BackgroundMigration::FixRubyObjectInAuditEvents.prepend_if_ee('EE::Gitlab::BackgroundMigration::FixRubyObjectInAuditEvents')