Add latest changes from gitlab-org/gitlab@master

13 files changed, 241 insertions, 68 deletions

diff --git a/app/models/concerns/from_set_operator.rb b/app/models/concerns/from_set_operator.rb
index ce3a83e9fa1..56b788eb1ab 100644
--- a/app/models/concerns/from_set_operator.rb
+++ b/app/models/concerns/from_set_operator.rb
@@ -10,7 +10,9 @@ module FromSetOperator
 
     raise "Trying to redefine method '#{method(method_name)}'" if methods.include?(method_name)
 
-    define_method(method_name) do |members, remove_duplicates: true, remove_order: true, alias_as: table_name|
+    define_method(method_name) do |*members, remove_duplicates: true, remove_order: true, alias_as: table_name|
+      members = flatten_ar_array(members)
+
       operator_sql =
         if members.any?
           operator.new(members, remove_duplicates: remove_duplicates, remove_order: remove_order).to_sql
@@ -20,5 +22,26 @@ module FromSetOperator
 
       from(Arel.sql("(#{operator_sql}) #{alias_as}"))
     end
+
+    # Array#flatten with ActiveRecord::Relation items will load the ActiveRecord::Relation.
+    # Therefore we need to roll our own flatten method.
+    unless method_defined?(:flatten_ar_array) # rubocop:disable Style/GuardClause
+      define_method :flatten_ar_array do |ary|
+        arrays = ary.dup
+        result = []
+
+        until arrays.empty?
+          item = arrays.shift
+          if item.is_a?(Array)
+            arrays.concat(item.dup)
+          else
+            result.push(item)
+          end
+        end
+
+        result
+      end
+      private :flatten_ar_array
+    end
   end
 end
diff --git a/app/services/projects/alerting/notify_service.rb b/app/services/projects/alerting/notify_service.rb
index c21a61bcb52..408d9dd24f2 100644
--- a/app/services/projects/alerting/notify_service.rb
+++ b/app/services/projects/alerting/notify_service.rb
@@ -2,14 +2,13 @@
 
 module Projects
   module Alerting
-    class NotifyService
+    class NotifyService < ::BaseProjectService
       extend ::Gitlab::Utils::Override
       include ::AlertManagement::AlertProcessing
       include ::AlertManagement::Responses
 
-      def initialize(project, payload)
-        @project = project
-        @payload = payload
+      def initialize(project, params)
+        super(project: project, params: params.to_h)
       end
 
       def execute(token, integration = nil)
@@ -29,10 +28,11 @@ module Projects
 
       private
 
-      attr_reader :project, :payload, :integration
+      attr_reader :integration
+      alias_method :payload, :params
 
       def valid_payload_size?
-        Gitlab::Utils::DeepSize.new(payload.to_h).valid?
+        Gitlab::Utils::DeepSize.new(params).valid?
       end
 
       override :alert_source
diff --git a/app/services/projects/prometheus/alerts/notify_service.rb b/app/services/projects/prometheus/alerts/notify_service.rb
index 6265a74fad2..9f260345937 100644
--- a/app/services/projects/prometheus/alerts/notify_service.rb
+++ b/app/services/projects/prometheus/alerts/notify_service.rb
@@ -3,9 +3,8 @@
 module Projects
   module Prometheus
     module Alerts
-      class NotifyService
+      class NotifyService < ::BaseProjectService
         include Gitlab::Utils::StrongMemoize
-        include ::IncidentManagement::Settings
         include ::AlertManagement::Responses
 
         # This set of keys identifies a payload as a valid Prometheus
@@ -26,14 +25,13 @@ module Projects
         # https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6086
         PROCESS_MAX_ALERTS = 100
 
-        def initialize(project, payload)
-          @project = project
-          @payload = payload
+        def initialize(project, params)
+          super(project: project, params: params.to_h)
         end
 
         def execute(token, integration = nil)
           return bad_request unless valid_payload_size?
-          return unprocessable_entity unless self.class.processable?(payload)
+          return unprocessable_entity unless self.class.processable?(params)
           return unauthorized unless valid_alert_manager_token?(token, integration)
 
           truncate_alerts! if max_alerts_exceeded?
@@ -53,10 +51,8 @@ module Projects
 
         private
 
-        attr_reader :project, :payload
-
         def valid_payload_size?
-          Gitlab::Utils::DeepSize.new(payload.to_h).valid?
+          Gitlab::Utils::DeepSize.new(params).valid?
         end
 
         def max_alerts_exceeded?
@@ -75,11 +71,11 @@ module Projects
             }
           )
 
-          payload['alerts'] = alerts.first(PROCESS_MAX_ALERTS)
+          params['alerts'] = alerts.first(PROCESS_MAX_ALERTS)
         end
 
         def alerts
-          payload['alerts']
+          params['alerts']
         end
 
         def valid_alert_manager_token?(token, integration)
@@ -152,7 +148,7 @@ module Projects
         def process_prometheus_alerts
           alerts.map do |alert|
             AlertManagement::ProcessPrometheusAlertService
-              .new(project, alert.to_h)
+              .new(project, alert)
               .execute
           end
         end
diff --git a/db/post_migrate/20220820221036_update_tmp_non_migrated_index_on_container_repositories.rb b/db/post_migrate/20220820221036_update_tmp_non_migrated_index_on_container_repositories.rb
new file mode 100644
index 00000000000..eea58ad7951
--- /dev/null
+++ b/db/post_migrate/20220820221036_update_tmp_non_migrated_index_on_container_repositories.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class UpdateTmpNonMigratedIndexOnContainerRepositories < Gitlab::Database::Migration[2.0]
+  disable_ddl_transaction!
+
+  NEW_INDEX_NAME = 'tmp_index_container_repos_on_non_migrated'
+  OLD_INDEX_NAME = 'tmp_idx_container_repos_on_non_migrated'
+  MIGRATION_PHASE_1_ENDED_AT = '2022-01-23'
+
+  def up
+    add_concurrent_index :container_repositories,
+                         [:project_id, :id],
+                         name: NEW_INDEX_NAME,
+                         where: "migration_state != 'import_done'"
+    remove_concurrent_index_by_name :container_repositories, OLD_INDEX_NAME
+  end
+
+  def down
+    add_concurrent_index :container_repositories,
+                         [:project_id, :id],
+                         name: OLD_INDEX_NAME,
+                         where: "migration_state != 'import_done' AND created_at < '#{MIGRATION_PHASE_1_ENDED_AT}'"
+    remove_concurrent_index_by_name :container_repositories, NEW_INDEX_NAME
+  end
+end
diff --git a/db/post_migrate/20220822071909_remove_other_role_from_user_details.rb b/db/post_migrate/20220822071909_remove_other_role_from_user_details.rb
new file mode 100644
index 00000000000..a0177bf2605
--- /dev/null
+++ b/db/post_migrate/20220822071909_remove_other_role_from_user_details.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class RemoveOtherRoleFromUserDetails < Gitlab::Database::Migration[2.0]
+  enable_lock_retries!
+
+  def change
+    remove_column :user_details, :other_role, :text
+  end
+end
diff --git a/db/schema_migrations/20220820221036 b/db/schema_migrations/20220820221036
new file mode 100644
index 00000000000..6f7c4059487
--- /dev/null
+++ b/db/schema_migrations/20220820221036
@@ -0,0 +1 @@
+16825936e8e6a4f0a1f001a83ecf81f180ee2eb15589eebe821fee2706456cef
\ No newline at end of file
diff --git a/db/schema_migrations/20220822071909 b/db/schema_migrations/20220822071909
new file mode 100644
index 00000000000..fd8af68d1ee
--- /dev/null
+++ b/db/schema_migrations/20220822071909
@@ -0,0 +1 @@
+60a72830780190214d6c86fc2d07dc0fc138f6cc258689c1d106bb456b130047
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index b291dfe7591..98622c25058 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -21887,7 +21887,6 @@ CREATE TABLE user_details (
     job_title character varying(200) DEFAULT ''::character varying NOT NULL,
     bio character varying(255) DEFAULT ''::character varying NOT NULL,
     webauthn_xid text,
-    other_role text,
     provisioned_by_group_id bigint,
     pronouns text,
     pronunciation text,
@@ -21896,7 +21895,6 @@ CREATE TABLE user_details (
     requires_credit_card_verification boolean DEFAULT false NOT NULL,
     CONSTRAINT check_245664af82 CHECK ((char_length(webauthn_xid) <= 100)),
     CONSTRAINT check_a73b398c60 CHECK ((char_length(phone) <= 50)),
-    CONSTRAINT check_b132136b01 CHECK ((char_length(other_role) <= 100)),
     CONSTRAINT check_eeeaf8d4f0 CHECK ((char_length(pronouns) <= 50)),
     CONSTRAINT check_f932ed37db CHECK ((char_length(pronunciation) <= 255))
 );
@@ -30633,14 +30631,14 @@ CREATE UNIQUE INDEX taggings_idx ON taggings USING btree (tag_id, taggable_id, t
 
 CREATE UNIQUE INDEX term_agreements_unique_index ON term_agreements USING btree (user_id, term_id);
 
-CREATE INDEX tmp_idx_container_repos_on_non_migrated ON container_repositories USING btree (project_id, id) WHERE ((migration_state <> 'import_done'::text) AND (created_at < '2022-01-23 00:00:00'::timestamp without time zone));
-
 CREATE INDEX tmp_index_ci_job_artifacts_on_expire_at_where_locked_unknown ON ci_job_artifacts USING btree (expire_at, job_id) WHERE ((locked = 2) AND (expire_at IS NOT NULL));
 
 CREATE INDEX tmp_index_ci_job_artifacts_on_id_where_trace_and_expire_at ON ci_job_artifacts USING btree (id) WHERE ((file_type = 3) AND (expire_at = ANY (ARRAY['2021-04-22 00:00:00+00'::timestamp with time zone, '2021-05-22 00:00:00+00'::timestamp with time zone, '2021-06-22 00:00:00+00'::timestamp with time zone, '2022-01-22 00:00:00+00'::timestamp with time zone, '2022-02-22 00:00:00+00'::timestamp with time zone, '2022-03-22 00:00:00+00'::timestamp with time zone, '2022-04-22 00:00:00+00'::timestamp with time zone])));
 
 CREATE INDEX tmp_index_cis_vulnerability_reads_on_id ON vulnerability_reads USING btree (id) WHERE (report_type = 7);
 
+CREATE INDEX tmp_index_container_repos_on_non_migrated ON container_repositories USING btree (project_id, id) WHERE (migration_state <> 'import_done'::text);
+
 CREATE INDEX tmp_index_container_repositories_on_id_migration_state ON container_repositories USING btree (id, migration_state);
 
 CREATE INDEX tmp_index_for_namespace_id_migration_on_group_members ON members USING btree (id) WHERE ((member_namespace_id IS NULL) AND ((type)::text = 'GroupMember'::text));
diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb
index 6f475fa8d74..c4464666020 100644
--- a/lib/api/internal/base.rb
+++ b/lib/api/internal/base.rb
@@ -133,11 +133,6 @@ module API
           'Could not find a user for the given key' unless actor.user
         end
 
-        # TODO: backwards compatibility; remove after https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/454 is merged
-        def two_factor_otp_check
-          { success: false, message: 'Feature is not available' }
-        end
-
         def two_factor_manual_otp_check
           { success: false, message: 'Feature is not available' }
         end
@@ -339,13 +334,6 @@ module API
           end
         end
 
-        # TODO: backwards compatibility; remove after https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/454 is merged
-        post '/two_factor_otp_check', feature_category: :authentication_and_authorization do
-          status 200
-
-          two_factor_manual_otp_check
-        end
-
         post '/two_factor_push_otp_check', feature_category: :authentication_and_authorization do
           status 200
 
diff --git a/lib/gitlab/database/lock_writes_manager.rb b/lib/gitlab/database/lock_writes_manager.rb
index cd483d616bb..fe75cd763b4 100644
--- a/lib/gitlab/database/lock_writes_manager.rb
+++ b/lib/gitlab/database/lock_writes_manager.rb
@@ -5,42 +5,63 @@ module Gitlab
     class LockWritesManager
       TRIGGER_FUNCTION_NAME = 'gitlab_schema_prevent_write'
 
-      def initialize(table_name:, connection:, database_name:, logger: nil)
+      def initialize(table_name:, connection:, database_name:, logger: nil, dry_run: false)
         @table_name = table_name
         @connection = connection
         @database_name = database_name
         @logger = logger
+        @dry_run = dry_run
+      end
+
+      def table_locked_for_writes?(table_name)
+        query = <<~SQL
+            SELECT COUNT(*) from information_schema.triggers
+            WHERE event_object_table = '#{table_name}'
+            AND trigger_name = '#{write_trigger_name(table_name)}'
+        SQL
+
+        connection.select_value(query) == 3
       end
 
       def lock_writes
+        if table_locked_for_writes?(table_name)
+          logger&.info "Skipping lock_writes, because #{table_name} is already locked for writes"
+          return
+        end
+
         logger&.info "Database: '#{database_name}', Table: '#{table_name}': Lock Writes".color(:yellow)
-        sql = <<-SQL
-          DROP TRIGGER IF EXISTS #{write_trigger_name(table_name)} ON #{table_name};
+        sql_statement = <<~SQL
           CREATE TRIGGER #{write_trigger_name(table_name)}
             BEFORE INSERT OR UPDATE OR DELETE OR TRUNCATE
             ON #{table_name}
             FOR EACH STATEMENT EXECUTE FUNCTION #{TRIGGER_FUNCTION_NAME}();
         SQL
 
-        with_retries(connection) do
-          connection.execute(sql)
-        end
+        execute_sql_statement(sql_statement)
       end
 
       def unlock_writes
         logger&.info "Database: '#{database_name}', Table: '#{table_name}': Allow Writes".color(:green)
-        sql = <<-SQL
-          DROP TRIGGER IF EXISTS #{write_trigger_name(table_name)} ON #{table_name}
+        sql_statement = <<~SQL
+          DROP TRIGGER IF EXISTS #{write_trigger_name(table_name)} ON #{table_name};
         SQL
 
-        with_retries(connection) do
-          connection.execute(sql)
-        end
+        execute_sql_statement(sql_statement)
       end
 
       private
 
-      attr_reader :table_name, :connection, :database_name, :logger
+      attr_reader :table_name, :connection, :database_name, :logger, :dry_run
+
+      def execute_sql_statement(sql)
+        if dry_run
+          logger&.info sql
+        else
+          with_retries(connection) do
+            connection.execute(sql)
+          end
+        end
+      end
 
       def with_retries(connection, &block)
         with_statement_timeout_retries do
diff --git a/spec/lib/gitlab/database/lock_writes_manager_spec.rb b/spec/lib/gitlab/database/lock_writes_manager_spec.rb
index eb527d492cf..b1cc8add55a 100644
--- a/spec/lib/gitlab/database/lock_writes_manager_spec.rb
+++ b/spec/lib/gitlab/database/lock_writes_manager_spec.rb
@@ -6,13 +6,15 @@ RSpec.describe Gitlab::Database::LockWritesManager do
   let(:connection) { ApplicationRecord.connection }
   let(:test_table) { '_test_table' }
   let(:logger) { instance_double(Logger) }
+  let(:dry_run) { false }
 
   subject(:lock_writes_manager) do
     described_class.new(
       table_name: test_table,
       connection: connection,
       database_name: 'main',
-      logger: logger
+      logger: logger,
+      dry_run: dry_run
     )
   end
 
@@ -27,6 +29,16 @@ RSpec.describe Gitlab::Database::LockWritesManager do
     SQL
   end
 
+  describe "#table_locked_for_writes?" do
+    it 'returns false for a table that is not locked for writes' do
+      expect(subject.table_locked_for_writes?(test_table)).to eq(false)
+    end
+
+    it 'returns true for a table that is locked for writes' do
+      expect { subject.lock_writes }.to change { subject.table_locked_for_writes?(test_table) }.from(false).to(true)
+    end
+  end
+
   describe '#lock_writes' do
     it 'prevents any writes on the table' do
       subject.lock_writes
@@ -84,11 +96,57 @@ RSpec.describe Gitlab::Database::LockWritesManager do
         subject.lock_writes
       end.to raise_error(ActiveRecord::QueryCanceled)
     end
+
+    it 'skips the operation if the table is already locked for writes' do
+      subject.lock_writes
+
+      expect(logger).to receive(:info).with("Skipping lock_writes, because #{test_table} is already locked for writes")
+      expect(connection).not_to receive(:execute).with(/CREATE TRIGGER/)
+
+      expect do
+        subject.lock_writes
+      end.not_to change {
+        number_of_triggers_on(connection, test_table)
+      }
+    end
+
+    context 'when running in dry_run mode' do
+      let(:dry_run) { true }
+
+      it 'prints the sql statement to the logger' do
+        expect(logger).to receive(:info).with("Database: 'main', Table: '#{test_table}': Lock Writes")
+        expected_sql_statement = <<~SQL
+          CREATE TRIGGER gitlab_schema_write_trigger_for_#{test_table}
+            BEFORE INSERT OR UPDATE OR DELETE OR TRUNCATE
+            ON #{test_table}
+            FOR EACH STATEMENT EXECUTE FUNCTION gitlab_schema_prevent_write();
+        SQL
+        expect(logger).to receive(:info).with(expected_sql_statement)
+
+        subject.lock_writes
+      end
+
+      it 'does not lock the tables for writes' do
+        subject.lock_writes
+
+        expect do
+          connection.execute("delete from #{test_table}")
+          connection.execute("truncate #{test_table}")
+        end.not_to raise_error
+      end
+    end
   end
 
   describe '#unlock_writes' do
     before do
-      subject.lock_writes
+      # Locking the table without the considering the value of dry_run
+      described_class.new(
+        table_name: test_table,
+        connection: connection,
+        database_name: 'main',
+        logger: logger,
+        dry_run: false
+      ).lock_writes
     end
 
     it 'allows writing on the table again' do
@@ -114,6 +172,28 @@ RSpec.describe Gitlab::Database::LockWritesManager do
 
       subject.unlock_writes
     end
+
+    context 'when running in dry_run mode' do
+      let(:dry_run) { true }
+
+      it 'prints the sql statement to the logger' do
+        expect(logger).to receive(:info).with("Database: 'main', Table: '#{test_table}': Allow Writes")
+        expected_sql_statement = <<~SQL
+          DROP TRIGGER IF EXISTS gitlab_schema_write_trigger_for_#{test_table} ON #{test_table};
+        SQL
+        expect(logger).to receive(:info).with(expected_sql_statement)
+
+        subject.unlock_writes
+      end
+
+      it 'does not unlock the tables for writes' do
+        subject.unlock_writes
+
+        expect do
+          connection.execute("delete from #{test_table}")
+        end.to raise_error(ActiveRecord::StatementInvalid, /Table: "#{test_table}" is write protected/)
+      end
+    end
   end
 
   def number_of_triggers_on(connection, table_name)
diff --git a/spec/models/concerns/from_set_operator_spec.rb b/spec/models/concerns/from_set_operator_spec.rb
index 8ebbb5550c9..1ef0d1adb08 100644
--- a/spec/models/concerns/from_set_operator_spec.rb
+++ b/spec/models/concerns/from_set_operator_spec.rb
@@ -3,7 +3,22 @@
 require 'spec_helper'
 
 RSpec.describe FromSetOperator do
-  describe 'when set operator method already exists' do
+  let_it_be(:from_set_operator) do
+    Class.new do
+      extend FromSetOperator
+      define_set_operator Gitlab::SQL::Union
+
+      def table_name
+        'groups'
+      end
+
+      def from(*args)
+        ''
+      end
+    end
+  end
+
+  context 'when set operator method already exists' do
     let(:redefine_method) do
       Class.new do
         def self.from_union
@@ -17,4 +32,38 @@ RSpec.describe FromSetOperator do
 
     it { expect { redefine_method }.to raise_exception(RuntimeError) }
   end
+
+  context 'with members' do
+    let_it_be(:group1) { create :group }
+    let_it_be(:group2) { create :group }
+    let_it_be(:groups) do
+      [
+        Group.where(id: group1),
+        Group.where(id: group2)
+      ]
+    end
+
+    shared_examples 'set operator called with correct members' do
+      it do
+        expect(Gitlab::SQL::Union).to receive(:new).with(groups, anything).and_call_original
+        subject
+      end
+    end
+
+    context 'as array' do
+      subject { from_set_operator.new.from_union(groups) }
+
+      it_behaves_like 'set operator called with correct members'
+
+      it { expect { subject }.not_to make_queries }
+    end
+
+    context 'as multiple parameters' do
+      subject { from_set_operator.new.from_union(*groups) }
+
+      it_behaves_like 'set operator called with correct members'
+
+      it { expect { subject }.not_to make_queries }
+    end
+  end
 end
diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb
index 9faeadc8433..26849a8eea0 100644
--- a/spec/requests/api/internal/base_spec.rb
+++ b/spec/requests/api/internal/base_spec.rb
@@ -1502,24 +1502,6 @@ RSpec.describe API::Internal::Base do
     end
   end
 
-  describe 'POST /internal/two_factor_otp_check' do
-    let(:key_id) { key.id }
-    let(:otp) { '123456' }
-
-    subject do
-      post api('/internal/two_factor_otp_check'),
-           params: { key_id: key_id, otp_attempt: otp },
-           headers: gitlab_shell_internal_api_request_header
-    end
-
-    it 'is not available' do
-      subject
-
-      expect(json_response['success']).to be_falsey
-      expect(json_response['message']).to eq 'Feature is not available'
-    end
-  end
-
   describe 'POST /internal/two_factor_manual_otp_check' do
     let(:key_id) { key.id }
     let(:otp) { '123456' }