From 64dc3e2698b057b02979fe65a367288d60431a22 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Wed, 1 Jun 2022 14:40:08 +0200 Subject: [PATCH] Limit allowed commit range for Changelog generation Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/340941 We want to verify the commit range before fetching the commits to keep a good endpoint performance. --- .../repositories/changelog_service.rb | 25 +++++++++++++++++++ .../changelog_commits_limitation.yml | 8 ++++++ doc/administration/instance_limits.md | 8 ++++++ doc/api/repositories.md | 5 ++++ .../repositories/changelog_service_spec.rb | 22 ++++++++++++++++ 5 files changed, 68 insertions(+) create mode 100644 config/feature_flags/development/changelog_commits_limitation.yml diff --git a/app/services/repositories/changelog_service.rb b/app/services/repositories/changelog_service.rb index 67eaefc37a2eb3..4cfbf2f1fb0dcc 100644 --- a/app/services/repositories/changelog_service.rb +++ b/app/services/repositories/changelog_service.rb @@ -6,6 +6,19 @@ class ChangelogService DEFAULT_TRAILER = 'Changelog' DEFAULT_FILE = 'CHANGELOG.md' + # The maximum number of commits allowed to fetch in `from` and `to` range. + # + # This value is arbitrarily chosen. Increasing it means more Gitaly calls + # and more presure on Gitaly services. + # + # This number is 3x of the average number of commits per GitLab releases. + # Some examples for GitLab's own releases: + # + # * 13.6.0: 4636 commits + # * 13.5.0: 5912 commits + # * 13.4.0: 5541 commits + COMMITS_LIMIT = 15_000 + # The `project` specifies the `Project` to generate the changelog section # for. # @@ -75,6 +88,8 @@ def execute(commit_to_changelog: true) commits = ChangelogCommitsFinder.new(project: @project, from: from, to: @to) + verify_commit_range!(from, @to) + commits.each_page(@trailer) do |page| mrs = mrs_finder.execute(page) @@ -120,5 +135,15 @@ def start_of_commit_range(config) 'could be found to use instead' ) end + + def verify_commit_range!(from, to) + return unless Feature.enabled?(:changelog_commits_limitation, @project) + + _, commits_count = @project.repository.diverging_commit_count(from, to) + + if commits_count > COMMITS_LIMIT + raise Gitlab::Changelog::Error, "The commits range exceeds #{COMMITS_LIMIT} elements." + end + end end end diff --git a/config/feature_flags/development/changelog_commits_limitation.yml b/config/feature_flags/development/changelog_commits_limitation.yml new file mode 100644 index 00000000000000..3339fc7f9462ae --- /dev/null +++ b/config/feature_flags/development/changelog_commits_limitation.yml @@ -0,0 +1,8 @@ +--- +name: changelog_commits_limitation +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89032 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/364101 +milestone: '15.1' +type: development +group: group::source code +default_enabled: false diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index 2fc4849560152b..340f96fcb3491a 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -1039,3 +1039,11 @@ The [secure files API](../api/secure_files.md) enforces the following limits: - Files must be smaller than 5 MB. - Projects cannot have more than 100 secure files. + +## Changelog API limits + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89032) in GitLab 15.1 [with a flag](../administration/feature_flags.md) named `changelog_commits_limitation`. Disabled by default. + +The [changelog API](../api/repositories.md#add-changelog-data-to-a-changelog-file) enforces the following limits: + +- The commit range between `from` and `to` cannot exceed 15000 commits. diff --git a/doc/api/repositories.md b/doc/api/repositories.md index ec5c97e5b25b19..a0f13bcce9ebfa 100644 --- a/doc/api/repositories.md +++ b/doc/api/repositories.md @@ -327,6 +327,11 @@ GitLab treats trailers case-sensitively. If you set the `trailer` field to `Example`, GitLab _won't_ include commits that use the trailer `example`, `eXaMpLE`, or anything else that isn't _exactly_ `Example`. +WARNING: +The allowed commits range between `from` and `to` is limited to 15000 commits. To disable +this restriction, [turn off the feature flag](../administration/feature_flags.md) +`changelog_commits_limitation`. + If the `from` attribute is unspecified, GitLab uses the Git tag of the last stable version that came before the version specified in the `version` attribute. This requires that Git tag names follow a specific format, allowing diff --git a/spec/services/repositories/changelog_service_spec.rb b/spec/services/repositories/changelog_service_spec.rb index 9139ab753c73dc..94363265188993 100644 --- a/spec/services/repositories/changelog_service_spec.rb +++ b/spec/services/repositories/changelog_service_spec.rb @@ -164,6 +164,28 @@ expect { request.call(sha3) }.not_to exceed_query_limit(control.count) end + + context 'when commit range exceeds the limit' do + let(:service) { described_class.new(project, creator, version: '1.0.0', from: sha1) } + + before do + stub_const("#{described_class.name}::COMMITS_LIMIT", 2) + end + + it 'raises an exception' do + expect { service.execute(commit_to_changelog: false) }.to raise_error(Gitlab::Changelog::Error) + end + + context 'when feature flag is off' do + before do + stub_feature_flags(changelog_commits_limitation: false) + end + + it 'returns the changelog' do + expect(service.execute(commit_to_changelog: false)).to include('Title 1', 'Title 2', 'Title 3') + end + end + end end describe '#start_of_commit_range' do -- GitLab