Add latest changes from gitlab-org/gitlab@master

.gitlab/CODEOWNERS

@@ -36,7 +36,7 @@
 /ee/app/finders/ @gitlab-org/maintainers/database
 
 # Feature specific owners
-/ee/lib/gitlab/code_owners/ @reprazent
+/ee/lib/gitlab/code_owners/ @reprazent @kerrizor
 /ee/lib/ee/gitlab/auth/ldap/ @dblessing @mkozono
 /lib/gitlab/auth/ldap/ @dblessing @mkozono
 /lib/gitlab/ci/templates/ @nolith @zj

app/assets/javascripts/behaviors/markdown/render_mermaid.js

@@ -28,7 +28,6 @@ function importMermaidModule() {
 
       if (
         window.gon?.user_color_scheme === 'dark' &&
-        window.gon?.features?.webideDarkTheme &&
         // if on the Web IDE page
         document.querySelector('.ide')
       ) {

app/assets/javascripts/ide/components/ide.vue

@@ -57,7 +57,7 @@ export default {
       'editorTheme',
     ]),
     themeName() {
-      return this.glFeatures.webideDarkTheme && window.gon?.user_color_scheme;
+      return window.gon?.user_color_scheme;
     },
   },
   mounted() {

app/controllers/ide_controller.rb

@@ -6,10 +6,6 @@ class IdeController < ApplicationController
   include ClientsidePreviewCSP
   include StaticObjectExternalStorageCSP
 
-  before_action do
-    push_frontend_feature_flag(:webide_dark_theme)
-  end
-
   def index
     Gitlab::UsageDataCounters::WebIdeCounter.increment_views_count
   end

app/models/project_ci_cd_setting.rb

@@ -37,8 +37,6 @@ class ProjectCiCdSetting < ApplicationRecord
   private
 
   def set_default_git_depth
-    return unless Feature.enabled?(:ci_set_project_default_git_depth, default_enabled: true)
-
     self.default_git_depth ||= DEFAULT_GIT_DEPTH
   end
 end

app/services/ci/register_job_service.rb

@@ -85,8 +85,6 @@ module Ci
       # to make sure that this is properly handled by runner.
       Result.new(nil, false)
     rescue => ex
-      raise ex unless Feature.enabled?(:ci_doom_build, default_enabled: true)
-
       scheduler_failure!(build)
       track_exception_for_build(ex, build)
 

app/workers/gitlab/jira_import/import_issue_worker.rb

@@ -29,9 +29,11 @@ module Gitlab
 
       def create_issue(issue_attributes, project_id)
         label_ids = issue_attributes.delete('label_ids')
+        assignee_ids = issue_attributes.delete('assignee_ids')
         issue_id = insert_and_return_id(issue_attributes, Issue)
 
         label_issue(project_id, issue_id, label_ids)
+        assign_issue(project_id, issue_id, assignee_ids)
 
         issue_id
       end
@@ -49,6 +51,14 @@ module Gitlab
         Gitlab::Database.bulk_insert(LabelLink.table_name, label_link_attrs)
       end
 
+      def assign_issue(project_id, issue_id, assignee_ids)
+        return if assignee_ids.blank?
+
+        assignee_attrs = assignee_ids.map { |user_id| { issue_id: issue_id, user_id: user_id } }
+
+        Gitlab::Database.bulk_insert(IssueAssignee.table_name, assignee_attrs)
+      end
+
       def build_label_attrs(issue_id, label_id)
         time = Time.now
         {

db/migrate/20200215222507_drop_forked_project_links_fk.rb

@@ -8,20 +8,16 @@ class DropForkedProjectLinksFk < ActiveRecord::Migration[6.0]
   disable_ddl_transaction!
 
   def up
-    # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction
     with_lock_retries do
       remove_foreign_key_if_exists :forked_project_links, column: :forked_to_project_id
     end
-    # rubocop: enable Migration/WithLockRetriesWithoutDdlTransaction
   end
 
   def down
     unless foreign_key_exists?(:forked_project_links, :projects, column: :forked_to_project_id)
-      # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction
       with_lock_retries do
         add_foreign_key :forked_project_links, :projects, column: :forked_to_project_id, on_delete: :cascade, validate: false
       end
-      # rubocop: enable Migration/WithLockRetriesWithoutDdlTransaction
     end
 
     fk_name = concurrent_foreign_key_name(:forked_project_links, :forked_to_project_id, prefix: 'fk_rails_')

db/migrate/20200304024025_add_sprint_id_index_to_issues.rb

@@ -13,7 +13,7 @@ class AddSprintIdIndexToIssues < ActiveRecord::Migration[6.0]
   end
 
   def down
-    with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction
+    with_lock_retries do
       remove_foreign_key :issues, column: :sprint_id
     end
     remove_concurrent_index :issues, :sprint_id

db/migrate/20200304024042_add_sprint_id_index_to_merge_requests.rb

@@ -13,7 +13,7 @@ class AddSprintIdIndexToMergeRequests < ActiveRecord::Migration[6.0]
   end
 
   def down
-    with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction
+    with_lock_retries do
       remove_foreign_key :merge_requests, column: :sprint_id
     end
     remove_concurrent_index :merge_requests, :sprint_id

db/migrate/20200407171133_add_protected_tag_create_access_levels_user_id_foreign_key.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction
 class AddProtectedTagCreateAccessLevelsUserIdForeignKey < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 

db/migrate/20200408154331_add_protected_branch_merge_access_levels_user_id_foreign_key.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction
 class AddProtectedBranchMergeAccessLevelsUserIdForeignKey < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 

db/migrate/20200408154411_add_path_locks_user_id_foreign_key.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction
 class AddPathLocksUserIdForeignKey < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 

db/migrate/20200408154455_add_protected_branch_push_access_levels_user_id_foreign_key.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction
 class AddProtectedBranchPushAccessLevelsUserIdForeignKey < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 

db/migrate/20200408154604_add_u2f_registrations_user_id_foreign_key.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction
 class AddU2fRegistrationsUserIdForeignKey < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 

db/migrate/20200420172752_add_sprints_foreign_key_to_projects.rb

@@ -12,7 +12,7 @@ class AddSprintsForeignKeyToProjects < ActiveRecord::Migration[6.0]
   end
 
   def down
-    with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction
+    with_lock_retries do
       remove_foreign_key :sprints, column: :project_id
     end
   end

db/migrate/20200420172927_add_sprints_foreign_key_to_groups.rb

@@ -12,7 +12,7 @@ class AddSprintsForeignKeyToGroups < ActiveRecord::Migration[6.0]
   end
 
   def down
-    with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction
+    with_lock_retries do
       remove_foreign_key :sprints, column: :group_id
     end
   end

db/migrate/20200423081441_add_foreign_key_from_users_to_metrics_users_starred_dashboars.rb

@@ -11,7 +11,7 @@ class AddForeignKeyFromUsersToMetricsUsersStarredDashboars < ActiveRecord::Migra
   end
 
   def down
-    with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction
+    with_lock_retries do
       remove_foreign_key_if_exists :metrics_users_starred_dashboards, column: :user_id
     end
   end

db/migrate/20200423081519_add_foreign_key_from_projects_to_metrics_users_starred_dashboars.rb

@@ -11,7 +11,7 @@ class AddForeignKeyFromProjectsToMetricsUsersStarredDashboars < ActiveRecord::Mi
   end
 
   def down
-    with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction
+    with_lock_retries do
       remove_foreign_key_if_exists :metrics_users_starred_dashboards, column: :project_id
     end
   end

db/post_migrate/20200424043515_drop_namespaces_plan_id.rb

@@ -8,14 +8,14 @@ class DropNamespacesPlanId < ActiveRecord::Migration[6.0]
   disable_ddl_transaction!
 
   def up
-    with_lock_retries do # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction
+    with_lock_retries do
       remove_column :namespaces, :plan_id
     end
   end
 
   def down
     unless column_exists?(:namespaces, :plan_id)
-      with_lock_retries do # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction
+      with_lock_retries do
         add_column :namespaces, :plan_id, :integer
       end
     end

doc/development/code_review.md

@@ -319,7 +319,7 @@ Before taking the decision to merge:
 - Set the milestone.
 - Consider warnings and errors from danger bot, code quality, and other reports.
   Unless a strong case can be made for the violation, these should be resolved
-  before merge. A comment must to be posted if the MR is merged with any failed job.
+  before merging. A comment must to be posted if the MR is merged with any failed job.
 
 When ready to merge:
 

doc/development/migration_style_guide.md

@@ -327,6 +327,34 @@ def down
 end
 ```
 
+**Usage with `disable_ddl_transaction!`**
+
+Generally the `with_lock_retries` helper should work with `disabled_ddl_transaction!`. A custom RuboCop rule ensures that only allowed methods can be placed within the lock retries block.
+
+```ruby
+disable_ddl_transaction!
+
+def up
+  with_lock_retries do
+    add_column :users, :name, :text
+  end
+
+  add_text_limit :users, :name, 255 # Includes constraint validation (full table scan)
+end
+```
+
+The RuboCop rule generally allows standard Rails migration methods, listed below. This example will cause a rubocop offense:
+
+```ruby
+disabled_ddl_transaction!
+
+def up
+  with_lock_retries do
+    add_concurrent_index :users, :name
+  end
+end
+```
+
 ### When to use the helper method
 
 The `with_lock_retries` helper method can be used when you normally use
@@ -350,8 +378,6 @@ Example changes:
 - `change_column_default`
 - `create_table` / `drop_table`
 
-**Note:** `with_lock_retries` method **cannot** be used with `disable_ddl_transaction!`.
-
 **Note:** `with_lock_retries` method **cannot** be used within the `change` method, you must manually define the `up` and `down` methods to make the migration reversible.
 
 ### How the helper method works

lib/gitlab/ci/config/entry/trigger.rb

@@ -25,8 +25,7 @@ module Gitlab
             strategy :CrossProjectTrigger, if: -> (config) { !config.key?(:include) }
 
             strategy :SameProjectTrigger, if: -> (config) do
-              ::Feature.enabled?(:ci_parent_child_pipeline, default_enabled: true) &&
-                config.key?(:include)
+              config.key?(:include)
             end
 
             class CrossProjectTrigger < ::Gitlab::Config::Entry::Node
@@ -72,11 +71,7 @@ module Gitlab
 
             class UnknownStrategy < ::Gitlab::Config::Entry::Node
               def errors
-                if ::Feature.enabled?(:ci_parent_child_pipeline, default_enabled: true)
-                  ['config must specify either project or include']
-                else
-                  ['config must specify project']
-                end
+                ['config must specify either project or include']
               end
             end
           end

rubocop/cop/migration/with_lock_retries_disallowed_method.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      class WithLockRetriesDisallowedMethod < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        ALLOWED_MIGRATION_METHODS = %i[
+          create_table
+          drop_table
+          add_foreign_key
+          remove_foreign_key
+          add_column
+          remove_column
+          execute
+          change_column_default
+          remove_foreign_key_if_exists
+          remove_foreign_key_without_error
+          table_exists?
+          index_exists_by_name?
+          foreign_key_exists?
+          index_exists?
+          column_exists?
+        ].sort.freeze
+
+        MSG = "The method is not allowed to be called within the `with_lock_retries` block, the only allowed methods are: #{ALLOWED_MIGRATION_METHODS.join(', ')}"
+
+        def_node_matcher :send_node?, <<~PATTERN
+        send
+        PATTERN
+
+        def on_block(node)
+          block_body = node.body
+
+          return unless in_migration?(node)
+          return unless block_body
+          return unless node.method_name == :with_lock_retries
+
+          if send_node?(block_body)
+            check_node(block_body)
+          else
+            block_body.children.each { |n| check_node(n) }
+          end
+        end
+
+        def check_node(node)
+          return unless send_node?(node)
+
+          name = node.children[1]
+          add_offense(node, location: :expression) unless ALLOWED_MIGRATION_METHODS.include?(name)
+        end
+      end
+    end
+  end
+end

rubocop/cop/migration/with_lock_retries_without_ddl_transaction.rb

@@ -1,36 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../../migration_helpers'
-
-module RuboCop
-  module Cop
-    module Migration
-      # Cop that prevents usage of `with_lock_retries` with `disable_ddl_transaction!`
-      class WithLockRetriesWithoutDdlTransaction < RuboCop::Cop::Cop
-        include MigrationHelpers
-
-        MSG = '`with_lock_retries` cannot be used with disabled DDL transactions (`disable_ddl_transaction!`). ' \
-          'Please remove the `disable_ddl_transaction!` call from your migration.'.freeze
-
-        def_node_matcher :disable_ddl_transaction?, <<~PATTERN
-        (send _ :disable_ddl_transaction!)
-        PATTERN
-
-        def_node_matcher :with_lock_retries?, <<~PATTERN
-          (send _ :with_lock_retries)
-        PATTERN
-
-        def on_send(node)
-          return unless in_migration?(node)
-          return unless with_lock_retries?(node)
-
-          node.each_ancestor(:begin) do |begin_node|
-            disable_ddl_transaction_node = begin_node.children.find { |n| disable_ddl_transaction?(n) }
-
-            add_offense(node, location: :expression) if disable_ddl_transaction_node
-          end
-        end
-      end
-    end
-  end
-end

scripts/regenerate-schema

@@ -2,7 +2,7 @@
 
 # frozen_string_literal: true
 
-require 'net/http'
+require 'open3'
 require 'uri'
 
 class SchemaRegenerator
@@ -56,35 +56,15 @@ class SchemaRegenerator
   # Get clean schema from remote servers
   #
   # This script might run in CI, using a shallow clone, so to checkout
-  # the file, download it from the server.
+  # the file, fetch the target branch from the server.
   def remote_checkout_clean_schema
     return false unless project_url
+    return false unless target_project_url
 
-    uri = URI.join("#{project_url}/", 'raw/', "#{merge_base}/", FILENAME)
+    run %Q[git remote add target_project #{target_project_url}.git]
+    run %Q[git fetch target_project #{target_branch}:#{target_branch}]
 
-    download_schema(uri)
-  end
-
-  ##
-  # Download the schema from the given +uri+.
-  def download_schema(uri)
-    puts "Downloading #{uri}..."
-
-    Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
-      request = Net::HTTP::Get.new(uri.request_uri)
-      http.read_timeout = 500
-      http.request(request) do |response|
-        raise("Failed to download file: #{response.code} #{response.message}") if response.code.to_i != 200
-
-        File.open(FILENAME, 'w') do |io|
-          response.read_body do |chunk|
-            io.write(chunk)
-          end
-        end
-      end
-    end
-
-    true
+    local_checkout_clean_schema
   end
 
   ##
@@ -150,15 +130,17 @@ class SchemaRegenerator
   # When the command failed an exception is raised.
   def run(cmd)
     puts "\e[32m$ #{cmd}\e[37m"
-    ret = system(cmd)
-    puts "\e[0m"
-    raise("Command failed") unless ret
+    stdout_str, stderr_str, status = Open3.capture3(cmd)
+    puts "#{stdout_str}#{stderr_str}\e[0m"
+    raise("Command failed: #{stderr_str}") unless status.success?
+
+    stdout_str
   end
 
   ##
   # Return the base commit between source and target branch.
   def merge_base
-    @merge_base ||= `git merge-base #{target_branch} #{source_ref}`.chomp
+    @merge_base ||= run("git merge-base #{target_branch} #{source_ref}").chomp
   end
 
   ##
@@ -179,11 +161,17 @@ class SchemaRegenerator
   end
 
   ##
-  # Return the project URL from CI environment variable.
+  # Return the source project URL from CI environment variable.
   def project_url
     ENV['CI_PROJECT_URL']
   end
 
+  ##
+  # Return the target project URL from CI environment variable.
+  def target_project_url
+    ENV['CI_MERGE_REQUEST_PROJECT_URL']
+  end
+
   ##
   # Return whether the script is running from CI
   def ci?

spec/lib/gitlab/ci/config/entry/trigger_spec.rb

@@ -114,19 +114,6 @@ describe Gitlab::Ci::Config::Entry::Trigger do
             .to match /config contains unknown keys: branch/
         end
       end
-
-      context 'when feature flag is off' do
-        before do
-          stub_feature_flags(ci_parent_child_pipeline: false)
-        end
-
-        let(:config) { { include: 'path/to/config.yml' } }
-
-        it 'is returns an error if include is used' do
-          expect(subject.errors.first)
-            .to match /config must specify project/
-        end
-      end
     end
 
     context 'when config contains unknown keys' do

spec/models/project_ci_cd_setting_spec.rb

@@ -54,17 +54,5 @@ describe ProjectCiCdSetting do
 
       expect(project.reload.ci_cd_settings.default_git_depth).to eq(0)
     end
-
-    context 'when feature flag :ci_set_project_default_git_depth is disabled' do
-      let(:project) { create(:project) }
-
-      before do
-        stub_feature_flags(ci_set_project_default_git_depth: { enabled: false } )
-      end
-
-      it 'does not set default value for new records' do
-        expect(project.ci_cd_settings.default_git_depth).to eq(nil)
-      end
-    end
   end
 end

spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb

@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../../rubocop/cop/migration/with_lock_retries_disallowed_method'
+
+describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'in migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+    end
+
+    it 'registers an offense when `with_lock_retries` block has disallowed method' do
+      inspect_source('def change; with_lock_retries { disallowed_method }; end')
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.map(&:line)).to eq([1])
+      end
+    end
+
+    it 'registers an offense when `with_lock_retries` block has disallowed methods' do
+      source = <<~HEREDOC
+      def change
+        with_lock_retries do
+          disallowed_method
+
+          create_table do |t|
+            t.text :text
+          end
+
+          other_disallowed_method
+
+          add_column :users, :name
+        end
+      end
+      HEREDOC
+
+      inspect_source(source)
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(2)
+        expect(cop.offenses.map(&:line)).to eq([3, 9])
+      end
+    end
+
+    it 'registers no offense when `with_lock_retries` has only allowed method' do
+      inspect_source('def up; with_lock_retries { add_foreign_key :foo, :bar }; end')
+
+      expect(cop.offenses.size).to eq(0)
+    end
+  end
+
+  context 'outside of migration' do
+    it 'registers no offense' do
+      inspect_source('def change; with_lock_retries { disallowed_method }; end')
+
+      expect(cop.offenses.size).to eq(0)
+    end
+  end
+end

spec/rubocop/cop/migration/with_lock_retries_without_ddl_transaction_spec.rb

@@ -1,46 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-require 'rubocop'
-require 'rubocop/rspec/support'
-
-require_relative '../../../../rubocop/cop/migration/with_lock_retries_without_ddl_transaction'
-
-describe RuboCop::Cop::Migration::WithLockRetriesWithoutDdlTransaction do
-  include CopHelper
-
-  let(:valid_source) { 'class MigrationClass < ActiveRecord::Migration[6.0]; def up; with_lock_retries {}; end; end' }
-  let(:invalid_source) { 'class MigrationClass < ActiveRecord::Migration[6.0]; disable_ddl_transaction!; def up; with_lock_retries {}; end; end' }
-
-  subject(:cop) { described_class.new }
-
-  context 'in migration' do
-    before do
-      allow(cop).to receive(:in_migration?).and_return(true)
-    end
-
-    it 'registers an offense when `with_lock_retries` is used with `disable_ddl_transaction!` method' do
-      inspect_source(invalid_source)
-
-      aggregate_failures do
-        expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.map(&:line)).to eq([1])
-      end
-    end
-
-    it 'registers no offense when `with_lock_retries` is used inside an `up` method' do
-      inspect_source(valid_source)
-
-      expect(cop.offenses.size).to eq(0)
-    end
-  end
-
-  context 'outside of migration' do
-    it 'registers no offense' do
-      inspect_source(invalid_source)
-
-      expect(cop.offenses.size).to eq(0)
-    end
-  end
-end

spec/workers/gitlab/jira_import/import_issue_worker_spec.rb

@@ -19,9 +19,12 @@ describe Gitlab::JiraImport::ImportIssueWorker do
   subject { described_class.new }
 
   describe '#perform', :clean_gitlab_redis_cache do
+    let(:assignee_ids) { [user.id] }
     let(:issue_attrs) do
       build(:issue, project_id: project.id, title: 'jira issue')
-        .as_json.merge('label_ids' => [jira_issue_label_1.id, jira_issue_label_2.id]).compact
+        .as_json.merge(
+          'label_ids' => [jira_issue_label_1.id, jira_issue_label_2.id], 'assignee_ids' => assignee_ids
+        ).compact
     end
 
     context 'when any exception raised while inserting to DB' do
@@ -67,6 +70,23 @@ describe Gitlab::JiraImport::ImportIssueWorker do
           expect(issue.title).to eq('jira issue')
           expect(issue.project).to eq(project)
           expect(issue.labels).to match_array([label, jira_issue_label_1, jira_issue_label_2])
+          expect(issue.assignees).to eq([user])
+        end
+
+        context 'when assignee_ids is nil' do
+          let(:assignee_ids) { nil }
+
+          it 'creates an issue without assignee' do
+            expect(Issue.last.assignees).to be_empty
+          end
+        end
+
+        context 'when assignee_ids is an empty array' do
+          let(:assignee_ids) { [] }
+
+          it 'creates an issue without assignee' do
+            expect(Issue.last.assignees).to be_empty
+          end
         end
       end
     end