From fba372a39a876351cfa50a4d24465e1eaaa3b3da Mon Sep 17 00:00:00 2001
From: Maxime Orefice <morefice@gitlab.com>
Date: Tue, 6 Jul 2021 15:18:51 -0400
Subject: [PATCH] Add cop to prevent index creation on forbidden table
This commit adds a new custom cop allowing us to prevent
index creation on a given table. The first table forbidden
is ci_builds.
---
.rubocop.yml | 6 +++
.../cop/migration/prevent_index_creation.rb | 41 +++++++++++++++
.../migration/prevent_index_creation_spec.rb | 50 +++++++++++++++++++
3 files changed, 97 insertions(+)
create mode 100644 rubocop/cop/migration/prevent_index_creation.rb
create mode 100644 spec/rubocop/cop/migration/prevent_index_creation_spec.rb
diff --git a/.rubocop.yml b/.rubocop.yml
index a26e9ab986b2..981b71e582ff 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -607,6 +607,12 @@ Migration/CreateTableWithForeignKeys:
Exclude:
- !ruby/regexp /\Adb\/(?:post_)?migrate\/(?:201[0-9]\d+|20200[0-8][0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9])_.+\.rb\z/
+Migration/PreventIndexCreation:
+ Exclude:
+ - !ruby/regexp /\Adb\/(post_)?migrate\/201.*\.rb\z/
+ - !ruby/regexp /\Adb\/(post_)?migrate\/2020.*\.rb\z/
+ - !ruby/regexp /\Adb\/(post_)?migrate\/20210[1-6].*\.rb\z/
+
Gitlab/RailsLogger:
Exclude:
- 'spec/**/*.rb'
diff --git a/rubocop/cop/migration/prevent_index_creation.rb b/rubocop/cop/migration/prevent_index_creation.rb
new file mode 100644
index 000000000000..c90f911d24e5
--- /dev/null
+++ b/rubocop/cop/migration/prevent_index_creation.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+require_relative '../../migration_helpers'
+
+module RuboCop
+ module Cop
+ module Migration
+ # Cop that checks if new indexes are introduced to forbidden tables.
+ class PreventIndexCreation < RuboCop::Cop::Cop
+ include MigrationHelpers
+
+ FORBIDDEN_TABLES = %i[ci_builds].freeze
+
+ MSG = "Adding new index to #{FORBIDDEN_TABLES.join(", ")} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886"
+
+ def_node_matcher :add_index?, <<~PATTERN
+ (send nil? :add_index (sym #forbidden_tables?) ...)
+ PATTERN
+
+ def_node_matcher :add_concurrent_index?, <<~PATTERN
+ (send nil? :add_concurrent_index (sym #forbidden_tables?) ...)
+ PATTERN
+
+ def forbidden_tables?(node)
+ FORBIDDEN_TABLES.include?(node)
+ end
+
+ def on_def(node)
+ return unless in_migration?(node)
+
+ node.each_descendant(:send) do |send_node|
+ add_offense(send_node, location: :selector) if offense?(send_node)
+ end
+ end
+
+ def offense?(node)
+ add_index?(node) || add_concurrent_index?(node)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/rubocop/cop/migration/prevent_index_creation_spec.rb b/spec/rubocop/cop/migration/prevent_index_creation_spec.rb
new file mode 100644
index 000000000000..a3965f54bbd5
--- /dev/null
+++ b/spec/rubocop/cop/migration/prevent_index_creation_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require_relative '../../../../rubocop/cop/migration/prevent_index_creation'
+
+RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do
+ subject(:cop) { described_class.new }
+
+ context 'when in migration' do
+ before do
+ allow(cop).to receive(:in_migration?).and_return(true)
+ end
+
+ context 'when adding an index to a forbidden table' do
+ it 'registers an offense when add_index is used' do
+ expect_offense(<<~RUBY)
+ def change
+ add_index :ci_builds, :protected
+ ^^^^^^^^^ Adding new index to ci_builds is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+ end
+ RUBY
+ end
+
+ it 'registers an offense when add_concurrent_index is used' do
+ expect_offense(<<~RUBY)
+ def change
+ add_concurrent_index :ci_builds, :protected
+ ^^^^^^^^^^^^^^^^^^^^ Adding new index to ci_builds is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+ end
+ RUBY
+ end
+ end
+
+ context 'when adding an index to a regular table' do
+ it 'does not register an offense' do
+ expect_no_offenses(<<~RUBY)
+ def change
+ add_index :ci_pipelines, :locked
+ end
+ RUBY
+ end
+ end
+ end
+
+ context 'when outside of migration' do
+ it 'does not register an offense' do
+ expect_no_offenses('def change; add_index :table, :column; end')
+ end
+ end
+end
--
GitLab