Add latest changes from gitlab-org/gitlab@12-7-stable-ee

Gemfile

@@ -132,9 +132,8 @@ gem 'seed-fu', '~> 2.3.7'
 gem 'elasticsearch-model', '~> 6.1'
 gem 'elasticsearch-rails', '~> 6.1', require: 'elasticsearch/rails/instrumentation'
 gem 'elasticsearch-api',   '~> 6.8'
-gem 'aws-sdk-core', '~> 3'
-gem 'aws-sdk-cloudformation', '~> 1'
-gem 'faraday_middleware-aws-sigv4'
+gem 'aws-sdk'
+gem 'faraday_middleware-aws-signers-v4'
 
 # Markdown and HTML processing
 gem 'html-pipeline', '~> 2.12'

Gemfile.lock

@@ -81,15 +81,13 @@ GEM
     attr_required (1.0.1)
     awesome_print (1.8.0)
     aws-eventstream (1.0.3)
-    aws-partitions (1.263.0)
-    aws-sdk-cloudformation (1.29.0)
-      aws-sdk-core (~> 3, >= 3.71.0)
-      aws-sigv4 (~> 1.1)
-    aws-sdk-core (3.88.0)
-      aws-eventstream (~> 1.0, >= 1.0.2)
-      aws-partitions (~> 1, >= 1.239.0)
-      aws-sigv4 (~> 1.1)
+    aws-sdk (2.11.374)
+      aws-sdk-resources (= 2.11.374)
+    aws-sdk-core (2.11.374)
+      aws-sigv4 (~> 1.0)
       jmespath (~> 1.0)
+    aws-sdk-resources (2.11.374)
+      aws-sdk-core (= 2.11.374)
     aws-sigv4 (1.1.0)
       aws-eventstream (~> 1.0, >= 1.0.2)
     axiom-types (0.1.1)
@@ -278,9 +276,9 @@ GEM
       faraday (~> 0.8)
     faraday_middleware (0.12.2)
       faraday (>= 0.7.4, < 1.0)
-    faraday_middleware-aws-sigv4 (0.3.0)
-      aws-sigv4 (~> 1.0)
-      faraday (>= 0.15)
+    faraday_middleware-aws-signers-v4 (0.1.7)
+      aws-sdk-resources (~> 2)
+      faraday (~> 0.9)
     faraday_middleware-multi_json (0.0.6)
       faraday_middleware
       multi_json
@@ -1144,8 +1142,7 @@ DEPENDENCIES
   atlassian-jwt (~> 0.2.0)
   attr_encrypted (~> 3.1.0)
   awesome_print
-  aws-sdk-cloudformation (~> 1)
-  aws-sdk-core (~> 3)
+  aws-sdk
   babosa (~> 1.0.2)
   base32 (~> 0.3.0)
   batch-loader (~> 1.4.0)
@@ -1191,7 +1188,7 @@ DEPENDENCIES
   escape_utils (~> 1.1)
   factory_bot_rails (~> 5.1.0)
   faraday (~> 0.12)
-  faraday_middleware-aws-sigv4
+  faraday_middleware-aws-signers-v4
   fast_blank
   ffaker (~> 2.10)
   flipper (~> 0.17.1)

app/controllers/projects_controller.rb

@@ -388,6 +388,7 @@ class ProjectsController < Projects::ApplicationController
       :merge_method,
       :initialize_with_readme,
       :autoclose_referenced_issues,
+      :suggestion_commit_message,
 
       project_feature_attributes: %i[
         builds_access_level

app/services/suggestions/apply_service.rb

@@ -96,7 +96,7 @@ module Suggestions
     end
 
     def suggestion_commit_message(project)
-      project.suggestion_commit_message || DEFAULT_SUGGESTION_COMMIT_MESSAGE
+      project.suggestion_commit_message.presence || DEFAULT_SUGGESTION_COMMIT_MESSAGE
     end
 
     def processed_suggestion_commit_message(suggestion)

changelogs/unreleased/198938-fix-invalid-issue-tracker-data.yml

+---
+title: Remove invalid data from issue_tracker_data table
+merge_request:
+author:
+type: fixed

changelogs/unreleased/fix-missing-apply-sugegstion-project-setting.yml

+---
+title: Add accidentally deleted project config for custom apply suggestions
+merge_request: 23687
+author: Fabio Huser
+type: fixed

changelogs/unreleased/id-fix-suggestions.yml

+---
+title: Fix applying the suggestions with an empty custom message
+merge_request: 24144
+author:
+type: fixed

changelogs/unreleased/sh-fix-trigger-check-rds.yml

+---
+title: Fix database permission check for triggers on Amazon RDS
+merge_request: 24035
+author:
+type: fixed

config/initializers/attr_encrypted_thread_safe.rb

+# frozen_string_literal: true
+
+# As of v3.1.0, attr_encrypted is not thread-safe because all instances share the same `encrypted_attributes`
+# This was fixed in https://github.com/attr-encrypted/attr_encrypted/commit/d4ca0e2073ca6ba5035997ce25f7fc0b4bfbe39e
+# but no release was made after that so we have to patch it ourselves here
+
+module AttrEncrypted
+  module InstanceMethods
+    def encrypted_attributes
+      @encrypted_attributes ||= begin
+        duplicated = {}
+        self.class.encrypted_attributes.map { |key, value| duplicated[key] = value.dup }
+        duplicated
+      end
+    end
+  end
+end

config/initializers/database_config.rb

@@ -10,6 +10,16 @@ def log_pool_size(db, previous_pool_size, current_pool_size)
   Gitlab::AppLogger.debug(log_message.join(' '))
 end
 
+Gitlab.ee do
+  # We need to initialize the Geo database before
+  # setting the Geo DB connection pool size.
+  if File.exist?(Rails.root.join('config/database_geo.yml'))
+    Rails.application.configure do
+      config.geo_database = config_for(:database_geo)
+    end
+  end
+end
+
 # When running on multi-threaded runtimes like Puma or Sidekiq,
 # set the number of threads per process as the minimum DB connection pool size.
 # This is to avoid connectivity issues as was documented here:

config/initializers/geo.rb

 # frozen_string_literal: true
 
 Gitlab.ee do
-  if File.exist?(Rails.root.join('config/database_geo.yml'))
-    Rails.application.configure do
-      config.geo_database = config_for(:database_geo)
-    end
-  end
-
-  begin
-    if Gitlab::Geo.connected? && Gitlab::Geo.primary?
-      Gitlab::Geo.current_node&.update_clone_url!
-    end
-  rescue => e
-    warn "WARNING: Unable to check/update clone_url_prefix for Geo: #{e}"
+  if Gitlab::Geo.connected? && Gitlab::Geo.primary?
+    Gitlab::Geo.current_node&.update_clone_url!
   end
+rescue => e
+  warn "WARNING: Unable to check/update clone_url_prefix for Geo: #{e}"
 end

db/post_migrate/20200127090233_remove_invalid_issue_tracker_data.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveInvalidIssueTrackerData < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    sql = "DELETE FROM issue_tracker_data WHERE \
+        (length(encrypted_issues_url) > 0 AND encrypted_issues_url_iv IS NULL) \
+        OR (length(encrypted_new_issue_url) > 0 AND encrypted_new_issue_url_iv IS NULL) \
+        OR (length(encrypted_project_url) > 0 AND encrypted_project_url_iv IS NULL)"
+
+    execute(sql)
+  end
+
+  def down
+    # We need to figure out why migrating data to issue_tracker_data table
+    # failed and then can recreate the data
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_01_23_155929) do
+ActiveRecord::Schema.define(version: 2020_01_27_090233) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"

lib/gitlab/database/grant.rb

@@ -3,23 +3,18 @@
 module Gitlab
   module Database
     # Model that can be used for querying permissions of a SQL user.
-    class Grant < ActiveRecord::Base
-      include FromUnion
-
-      self.table_name = 'information_schema.role_table_grants'
-
+    class Grant
       # Returns true if the current user can create and execute triggers on the
       # given table.
       def self.create_and_execute_trigger?(table)
         # We _must not_ use quote_table_name as this will produce double
         # quotes on PostgreSQL and for "has_table_privilege" we need single
         # quotes.
+        connection = ActiveRecord::Base.connection
         quoted_table = connection.quote(table)
 
         begin
-          from(nil)
-            .pluck(Arel.sql("has_table_privilege(#{quoted_table}, 'TRIGGER')"))
-            .first
+          connection.select_one("SELECT has_table_privilege(#{quoted_table}, 'TRIGGER')").present?
         rescue ActiveRecord::StatementInvalid
           # This error is raised when using a non-existing table name. In this
           # case we just want to return false as a user technically can't

spec/initializers/attr_encrypted_thread_safe_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AttrEncrypted do
+  describe '#encrypted_attributes' do
+    subject do
+      Class.new(ActiveRecord::Base) do
+        self.table_name = 'projects'
+
+        attr_accessor :encrypted_foo
+        attr_accessor :encrypted_foo_iv
+
+        attr_encrypted :foo, key: 'This is a key that is 256 bits!!'
+      end
+    end
+
+    it 'does not share state with other instances' do
+      instance = subject.new
+      instance.foo = 'bar'
+
+      another_instance = subject.new
+
+      expect(instance.encrypted_attributes[:foo][:operation]).to eq(:encrypting)
+      expect(another_instance.encrypted_attributes[:foo][:operation]).to be_nil
+    end
+  end
+end

spec/migrations/20200127090233_remove_invalid_issue_tracker_data_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200127090233_remove_invalid_issue_tracker_data.rb')
+
+describe RemoveInvalidIssueTrackerData, :migration do
+  let(:issue_tracker_data) { table(:issue_tracker_data) }
+  let(:services) { table(:services) }
+
+  let(:service) { services.create(id: 1) }
+  let(:data) do
+    {
+      service_id: service.id,
+      encrypted_issues_url: 'http:url.com',
+      encrypted_issues_url_iv: 'somevalue',
+      encrypted_new_issue_url: 'http:url.com',
+      encrypted_new_issue_url_iv: 'somevalue',
+      encrypted_project_url: 'username',
+      encrypted_project_url_iv: 'somevalue'
+    }
+  end
+
+  let!(:valid_data) { issue_tracker_data.create(data) }
+  let!(:empty_data) { issue_tracker_data.create(service_id: service.id) }
+  let!(:invalid_issues_url) do
+    data[:encrypted_issues_url_iv] = nil
+    issue_tracker_data.create(data)
+  end
+  let!(:missing_issues_url) do
+    data[:encrypted_issues_url] = ''
+    data[:encrypted_issues_url_iv] = nil
+    issue_tracker_data.create(data)
+  end
+  let!(:invalid_new_isue_url) do
+    data[:encrypted_new_issue_url_iv] = nil
+    issue_tracker_data.create(data)
+  end
+  let!(:missing_new_issue_url) do
+    data[:encrypted_new_issue_url] = ''
+    issue_tracker_data.create(data)
+  end
+  let!(:invalid_project_url) do
+    data[:encrypted_project_url_iv] = nil
+    issue_tracker_data.create(data)
+  end
+  let!(:missing_project_url) do
+    data[:encrypted_project_url] = nil
+    data[:encrypted_project_url_iv] = nil
+    issue_tracker_data.create(data)
+  end
+
+  it 'removes the invalid data' do
+    valid_data_records = [valid_data, empty_data, missing_issues_url, missing_new_issue_url, missing_project_url]
+
+    expect { migrate! }.to change { issue_tracker_data.count }.from(8).to(5)
+
+    expect(issue_tracker_data.all).to match_array(valid_data_records)
+  end
+end

spec/services/suggestions/apply_service_spec.rb

@@ -57,10 +57,22 @@ describe Suggestions::ApplyService do
       end
 
       context 'is not specified' do
-        let(:message) { nil }
+        let(:expected_value) { "Apply suggestion to files/ruby/popen.rb" }
 
-        it 'sets default commit message' do
-          expect(project.repository.commit.message).to eq("Apply suggestion to files/ruby/popen.rb")
+        context 'is nil' do
+          let(:message) { nil }
+
+          it 'sets default commit message' do
+            expect(project.repository.commit.message).to eq(expected_value)
+          end
+        end
+
+        context 'is an empty string' do
+          let(:message) { '' }
+
+          it 'sets default commit message' do
+            expect(project.repository.commit.message).to eq(expected_value)
+          end
         end
       end