From 965ff14b6fec206d81e2a94fceddd0eda3dfed3a Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Thu, 6 Sep 2018 17:56:06 -0300 Subject: [PATCH] Send max_patch_bytes to Gitaly via Gitaly::CommitDiffRequest We used to apply this limitation on GitLab when using Rugged. Now that we've shifted to Gitaly, this parameter needs to be sent via a RPC request. Currently this value is hardcoded on Gitaly. --- Gemfile | 2 +- Gemfile.lock | 4 ++-- Gemfile.rails5.lock | 4 ++-- .../osw-send-max-patch-bytes-to-gitaly.yml | 5 +++++ lib/gitlab/git/diff_collection.rb | 5 +++-- lib/gitlab/gitaly_client/commit_service.rb | 2 +- .../gitaly_client/commit_service_spec.rb | 18 ++++++++++++++++-- 7 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 changelogs/unreleased/osw-send-max-patch-bytes-to-gitaly.yml diff --git a/Gemfile b/Gemfile index 7b83c6d1178..ec1274f2d19 100644 --- a/Gemfile +++ b/Gemfile @@ -423,7 +423,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.113.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.117.0', require: 'gitaly' gem 'grpc', '~> 1.11.0' # Locked until https://github.com/google/protobuf/issues/4210 is closed diff --git a/Gemfile.lock b/Gemfile.lock index b9fa9c74919..e4bce2ed1f7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -276,7 +276,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.113.0) + gitaly-proto (0.117.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1038,7 +1038,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.113.0) + gitaly-proto (~> 0.117.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 0171c3564e3..1e9afeb47ff 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -279,7 +279,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.113.0) + gitaly-proto (0.117.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1047,7 +1047,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.113.0) + gitaly-proto (~> 0.117.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/changelogs/unreleased/osw-send-max-patch-bytes-to-gitaly.yml b/changelogs/unreleased/osw-send-max-patch-bytes-to-gitaly.yml new file mode 100644 index 00000000000..3c50448e3ff --- /dev/null +++ b/changelogs/unreleased/osw-send-max-patch-bytes-to-gitaly.yml @@ -0,0 +1,5 @@ +--- +title: Send max_patch_bytes to Gitaly via Gitaly::CommitDiffRequest +merge_request: 21575 +author: +type: other diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb index 219c69893ad..20dce8d0e06 100644 --- a/lib/gitlab/git/diff_collection.rb +++ b/lib/gitlab/git/diff_collection.rb @@ -11,7 +11,7 @@ module Gitlab delegate :max_files, :max_lines, :max_bytes, :safe_max_files, :safe_max_lines, :safe_max_bytes, to: :limits - def self.collection_limits(options = {}) + def self.limits(options = {}) limits = {} limits[:max_files] = options.fetch(:max_files, DEFAULT_LIMITS[:max_files]) limits[:max_lines] = options.fetch(:max_lines, DEFAULT_LIMITS[:max_lines]) @@ -19,13 +19,14 @@ module Gitlab limits[:safe_max_files] = [limits[:max_files], DEFAULT_LIMITS[:max_files]].min limits[:safe_max_lines] = [limits[:max_lines], DEFAULT_LIMITS[:max_lines]].min limits[:safe_max_bytes] = limits[:safe_max_files] * 5.kilobytes # Average 5 KB per file + limits[:max_patch_bytes] = Gitlab::Git::Diff::SIZE_LIMIT OpenStruct.new(limits) end def initialize(iterator, options = {}) @iterator = iterator - @limits = self.class.collection_limits(options) + @limits = self.class.limits(options) @enforce_limits = !!options.fetch(:limits, true) @expanded = !!options.fetch(:expanded, true) diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 6a97cd8ed17..aa5b4f94090 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -369,7 +369,7 @@ module Gitlab request_params[:ignore_whitespace_change] = options.fetch(:ignore_whitespace_change, false) request_params[:enforce_limits] = options.fetch(:limits, true) request_params[:collapse_diffs] = !options.fetch(:expanded, true) - request_params.merge!(Gitlab::Git::DiffCollection.collection_limits(options).to_h) + request_params.merge!(Gitlab::Git::DiffCollection.limits(options).to_h) request = Gitaly::CommitDiffRequest.new(request_params) response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request, timeout: GitalyClient.medium_timeout) diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 54f2ea33f90..bcdf12a00a0 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -19,7 +19,14 @@ describe Gitlab::GitalyClient::CommitService do right_commit_id: commit.id, collapse_diffs: false, enforce_limits: true, - **Gitlab::Git::DiffCollection.collection_limits.to_h + # Tests limitation parameters explicitly + max_files: 100, + max_lines: 5000, + max_bytes: 512000, + safe_max_files: 100, + safe_max_lines: 5000, + safe_max_bytes: 512000, + max_patch_bytes: 102400 ) expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_diff).with(request, kind_of(Hash)) @@ -37,7 +44,14 @@ describe Gitlab::GitalyClient::CommitService do right_commit_id: initial_commit.id, collapse_diffs: false, enforce_limits: true, - **Gitlab::Git::DiffCollection.collection_limits.to_h + # Tests limitation parameters explicitly + max_files: 100, + max_lines: 5000, + max_bytes: 512000, + safe_max_files: 100, + safe_max_lines: 5000, + safe_max_bytes: 512000, + max_patch_bytes: 102400 ) expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_diff).with(request, kind_of(Hash))