diff options
| -rw-r--r-- | Gemfile | 5 | ||||
| -rw-r--r-- | Gemfile.lock | 25 | ||||
| -rw-r--r-- | app/controllers/projects_controller.rb | 1 | ||||
| -rw-r--r-- | app/services/suggestions/apply_service.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/198938-fix-invalid-issue-tracker-data.yml | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/fix-missing-apply-sugegstion-project-setting.yml | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/id-fix-suggestions.yml | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/sh-fix-trigger-check-rds.yml | 5 | ||||
| -rw-r--r-- | config/initializers/attr_encrypted_thread_safe.rb | 17 | ||||
| -rw-r--r-- | config/initializers/database_config.rb | 10 | ||||
| -rw-r--r-- | config/initializers/geo.rb | 16 | ||||
| -rw-r--r-- | db/post_migrate/20200127090233_remove_invalid_issue_tracker_data.rb | 24 | ||||
| -rw-r--r-- | db/schema.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/database/grant.rb | 11 | ||||
| -rw-r--r-- | spec/initializers/attr_encrypted_thread_safe_spec.rb | 28 | ||||
| -rw-r--r-- | spec/migrations/20200127090233_remove_invalid_issue_tracker_data_spec.rb | 59 | ||||
| -rw-r--r-- | spec/services/suggestions/apply_service_spec.rb | 18 |
17 files changed, 196 insertions, 42 deletions
@@ -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' diff --git a/Gemfile.lock b/Gemfile.lock index d4dd0a37570..0733d49c3de 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 9d0901881d3..d39a4c373ff 100644 --- a/app/controllers/projects_controller.rb +++ b/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 diff --git a/app/services/suggestions/apply_service.rb b/app/services/suggestions/apply_service.rb index a6485e42bdb..479eed3ce57 100644 --- a/app/services/suggestions/apply_service.rb +++ b/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) diff --git a/changelogs/unreleased/198938-fix-invalid-issue-tracker-data.yml b/changelogs/unreleased/198938-fix-invalid-issue-tracker-data.yml new file mode 100644 index 00000000000..36bf6e3aa26 --- /dev/null +++ b/changelogs/unreleased/198938-fix-invalid-issue-tracker-data.yml @@ -0,0 +1,5 @@ +--- +title: Remove invalid data from issue_tracker_data table +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/fix-missing-apply-sugegstion-project-setting.yml b/changelogs/unreleased/fix-missing-apply-sugegstion-project-setting.yml new file mode 100644 index 00000000000..d084148fbd8 --- /dev/null +++ b/changelogs/unreleased/fix-missing-apply-sugegstion-project-setting.yml @@ -0,0 +1,5 @@ +--- +title: Add accidentally deleted project config for custom apply suggestions +merge_request: 23687 +author: Fabio Huser +type: fixed diff --git a/changelogs/unreleased/id-fix-suggestions.yml b/changelogs/unreleased/id-fix-suggestions.yml new file mode 100644 index 00000000000..3987a590e14 --- /dev/null +++ b/changelogs/unreleased/id-fix-suggestions.yml @@ -0,0 +1,5 @@ +--- +title: Fix applying the suggestions with an empty custom message +merge_request: 24144 +author: +type: fixed diff --git a/changelogs/unreleased/sh-fix-trigger-check-rds.yml b/changelogs/unreleased/sh-fix-trigger-check-rds.yml new file mode 100644 index 00000000000..99ae6039aaa --- /dev/null +++ b/changelogs/unreleased/sh-fix-trigger-check-rds.yml @@ -0,0 +1,5 @@ +--- +title: Fix database permission check for triggers on Amazon RDS +merge_request: 24035 +author: +type: fixed diff --git a/config/initializers/attr_encrypted_thread_safe.rb b/config/initializers/attr_encrypted_thread_safe.rb new file mode 100644 index 00000000000..be0bb56ffdc --- /dev/null +++ b/config/initializers/attr_encrypted_thread_safe.rb @@ -0,0 +1,17 @@ +# 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 diff --git a/config/initializers/database_config.rb b/config/initializers/database_config.rb index b5490fc4719..52897ad549d 100644 --- a/config/initializers/database_config.rb +++ b/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: diff --git a/config/initializers/geo.rb b/config/initializers/geo.rb index 4cc9fbf49b2..3278dc9e484 100644 --- a/config/initializers/geo.rb +++ b/config/initializers/geo.rb @@ -1,17 +1,9 @@ # 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 diff --git a/db/post_migrate/20200127090233_remove_invalid_issue_tracker_data.rb b/db/post_migrate/20200127090233_remove_invalid_issue_tracker_data.rb new file mode 100644 index 00000000000..6d64dd3d0d1 --- /dev/null +++ b/db/post_migrate/20200127090233_remove_invalid_issue_tracker_data.rb @@ -0,0 +1,24 @@ +# 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 diff --git a/db/schema.rb b/db/schema.rb index 12dd2a0adc8..528631c4b36 100644 --- a/db/schema.rb +++ b/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" diff --git a/lib/gitlab/database/grant.rb b/lib/gitlab/database/grant.rb index 1f47f320a29..7774dd9fffe 100644 --- a/lib/gitlab/database/grant.rb +++ b/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 diff --git a/spec/initializers/attr_encrypted_thread_safe_spec.rb b/spec/initializers/attr_encrypted_thread_safe_spec.rb new file mode 100644 index 00000000000..096b8b196b4 --- /dev/null +++ b/spec/initializers/attr_encrypted_thread_safe_spec.rb @@ -0,0 +1,28 @@ +# 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 diff --git a/spec/migrations/20200127090233_remove_invalid_issue_tracker_data_spec.rb b/spec/migrations/20200127090233_remove_invalid_issue_tracker_data_spec.rb new file mode 100644 index 00000000000..d1b1dfb1904 --- /dev/null +++ b/spec/migrations/20200127090233_remove_invalid_issue_tracker_data_spec.rb @@ -0,0 +1,59 @@ +# 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 diff --git a/spec/services/suggestions/apply_service_spec.rb b/spec/services/suggestions/apply_service_spec.rb index 84529af7187..b04c3278eaa 100644 --- a/spec/services/suggestions/apply_service_spec.rb +++ b/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 |