From b6e5f5df140a172682b630075e9c98fde6b87c01 Mon Sep 17 00:00:00 2001
From: Johannes Bauer <jobauer@gitlab.com>
Date: Thu, 6 Mar 2025 06:21:51 +0100
Subject: [PATCH] Extend Package Registry DB Table
---
app/finders/packages/helm/packages_finder.rb | 11 ++--
app/helpers/application_settings_helper.rb | 1 +
app/models/application_setting.rb | 10 +++-
.../application_setting_implementation.rb | 1 +
.../application_setting_package_registry.json | 5 ++
.../_package_registry.html.haml | 7 +--
.../_packages_limits_settings.html.haml | 21 ++++++++
config/routes/admin.rb | 1 +
doc/api/settings.md | 1 +
doc/development/packages/settings.md | 1 +
doc/user/packages/helm_repository/_index.md | 2 +-
.../_ee_package_registry.haml | 2 +-
locale/gitlab.pot | 14 +++++-
.../packages/helm/packages_finder_spec.rb | 23 +++++++--
spec/models/application_setting_spec.rb | 2 +
...packages_limits_settings.html.haml_spec.rb | 50 +++++++++++++++++++
16 files changed, 138 insertions(+), 14 deletions(-)
create mode 100644 app/views/admin/application_settings/_packages_limits_settings.html.haml
create mode 100644 spec/views/admin/application_settings/_packages_limits_settings.html.haml_spec.rb
diff --git a/app/finders/packages/helm/packages_finder.rb b/app/finders/packages/helm/packages_finder.rb
index 808c9f5eb1f35..91dd3b8005bfc 100644
--- a/app/finders/packages/helm/packages_finder.rb
+++ b/app/finders/packages/helm/packages_finder.rb
@@ -5,8 +5,6 @@ module Helm
class PackagesFinder
include ::Packages::FinderHelper
- MAX_PACKAGES_COUNT = 1000
-
def initialize(project, channel)
@project = project
@channel = channel
@@ -20,9 +18,16 @@ def execute
# we use a subquery to get unique packages and at the same time
# order + limit them.
::Packages::Package
- .limit_recent(MAX_PACKAGES_COUNT)
+ .limit_recent(max_packages_count)
.id_in(pkg_files.select(:package_id))
end
+
+ private
+
+ def max_packages_count
+ ::Gitlab::CurrentSettings.package_registry.fetch('helm_max_packages_count',
+ ::ApplicationSetting::DEFAULT_HELM_MAX_PACKAGES_COUNT)
+ end
end
end
end
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index ff75ee9c89509..4db587b6649ae 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -593,6 +593,7 @@ def visible_attributes
settings << :deactivate_dormant_users
settings << :deactivate_dormant_users_period
settings << :nuget_skip_metadata_url_validation
+ settings << :helm_max_packages_count
end
end
end
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 5f299cf243501..17adf898ff366 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -742,7 +742,15 @@ def self.kroki_formats_attributes
validates :importers, json_schema: { filename: "application_setting_importers" }
- jsonb_accessor :package_registry, nuget_skip_metadata_url_validation: [:boolean, { default: false }]
+ DEFAULT_HELM_MAX_PACKAGES_COUNT = 1000
+
+ jsonb_accessor :package_registry,
+ nuget_skip_metadata_url_validation: [:boolean, { default: false }],
+ helm_max_packages_count: [:integer, { default: DEFAULT_HELM_MAX_PACKAGES_COUNT }]
+
+ validates :helm_max_packages_count,
+ presence: true,
+ numericality: { only_integer: true, greater_than: 0 }
jsonb_accessor :oauth_provider, ropc_without_client_credentials: [:boolean, { default: true }]
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index 26603f08c9648..de0f6de43af87 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -309,6 +309,7 @@ def defaults # rubocop:disable Metrics/AbcSize
users_api_limit_gpg_keys: 120,
users_api_limit_gpg_key: 120,
nuget_skip_metadata_url_validation: false,
+ helm_max_packages_count: 1000,
ai_action_api_rate_limit: 160,
code_suggestions_api_rate_limit: 60,
require_personal_access_token_expiry: true,
diff --git a/app/validators/json_schemas/application_setting_package_registry.json b/app/validators/json_schemas/application_setting_package_registry.json
index bf2e67c6f4215..4597d6ab6fb6d 100644
--- a/app/validators/json_schemas/application_setting_package_registry.json
+++ b/app/validators/json_schemas/application_setting_package_registry.json
@@ -6,6 +6,11 @@
"nuget_skip_metadata_url_validation": {
"type": "boolean",
"description": "Indicates whether to skip metadata URL validation for the NuGet package"
+ },
+ "helm_max_packages_count": {
+ "type": ["integer", "null"],
+ "minimum": 1,
+ "description": "Maximum number of Helm packages that can be listed per channel. If null, defaults to 1000"
}
},
"additionalProperties": false
diff --git a/app/views/admin/application_settings/_package_registry.html.haml b/app/views/admin/application_settings/_package_registry.html.haml
index 2a703a6a01cea..48f8613f4ce96 100644
--- a/app/views/admin/application_settings/_package_registry.html.haml
+++ b/app/views/admin/application_settings/_package_registry.html.haml
@@ -3,15 +3,16 @@
id: 'js-package-settings',
expanded: expanded_by_default?) do |c|
- c.with_description do
- = s_('PackageRegistry|Configure package forwarding and package file size limits.')
+ = s_('PackageRegistry|Configure package forwarding, package limits, and package file size limits.')
- c.with_body do
= render_if_exists 'admin/application_settings/ee_package_registry'
= render 'admin/application_settings/nuget_skip_metadata_url_validation' unless Gitlab.ee?
+ = render 'admin/application_settings/packages_limits_settings'
.gl-mt-7
- %h4
+ %h4.gl-mb-1
= _('Package file size limits')
- %p
+ %p.gl-text-subtle
= _('Set limit to 0 to allow any file size.')
.scrolling-tabs-container.inner-page-scroll-tabs
- if @plans.size > 1
diff --git a/app/views/admin/application_settings/_packages_limits_settings.html.haml b/app/views/admin/application_settings/_packages_limits_settings.html.haml
new file mode 100644
index 0000000000000..e350806eb94b6
--- /dev/null
+++ b/app/views/admin/application_settings/_packages_limits_settings.html.haml
@@ -0,0 +1,21 @@
+.gl-mt-7
+ %h4.gl-mb-1
+ = _('Package limits')
+ %p.gl-text-subtle
+ = _('Setting high package limits can impact database performance. Consider the size of your instance when configuring these values.')
+ .scrolling-tabs-container.inner-page-scroll-tabs
+ .tab-content
+ = gitlab_ui_form_for @application_setting,
+ url: ci_cd_admin_application_settings_path(anchor: 'js-package-settings'),
+ method: :patch, html: { class: 'fieldset-form' }, data: { testid: 'package-limits-form' } do |f|
+ = form_errors(@application_setting)
+ %fieldset
+
+ .form-group
+ = f.label :helm_max_packages_count, _('Maximum number of Helm packages per channel'), class: 'label-bold'
+ = f.number_field :helm_max_packages_count,
+ class: 'form-control gl-form-input',
+ placeholder: ::ApplicationSetting::DEFAULT_HELM_MAX_PACKAGES_COUNT
+ .form-text.text-muted
+ = _('Maximum number of Helm packages that can be listed per channel. Must be at least 1.')
+ = f.submit _('Save changes'), pajamas_button: true
diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index a00f316e85cd0..39a2e44ba64b7 100644
--- a/config/routes/admin.rb
+++ b/config/routes/admin.rb
@@ -185,6 +185,7 @@
end
resources :plan_limits, only: :create
+ resource :packages_limits, only: :update
resources :labels
diff --git a/doc/api/settings.md b/doc/api/settings.md
index 33d6341d9ef94..f988b968bd576 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -764,6 +764,7 @@ to configure other related settings. These requirements are
| `duo_features_enabled` | boolean | no | Indicates whether GitLab Duo features are enabled for this instance. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144931) in GitLab 16.10. GitLab Self-Managed, Premium and Ultimate only. |
| `lock_duo_features_enabled` | boolean | no | Indicates whether the GitLab Duo features enabled setting is enforced for all subgroups. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144931) in GitLab 16.10. GitLab Self-Managed, Premium and Ultimate only. |
| `nuget_skip_metadata_url_validation` | boolean | no | Indicates whether to skip metadata URL validation for the NuGet package. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145887) in GitLab 17.0. |
+| `helm_max_packages_count` | integer | no | Maximum number of Helm packages that can be listed per channel. Must be at least 1. Default is 1000. |
| `require_admin_two_factor_authentication` | boolean | no | Allow administrators to require 2FA for all administrators on the instance. |
| `secret_push_protection_available` | boolean | no | Allow projects to enable secret push protection. This does not enable secret push protection. Ultimate only. |
diff --git a/doc/development/packages/settings.md b/doc/development/packages/settings.md
index 87ea11b88e1d9..fe2a56cc92c6f 100644
--- a/doc/development/packages/settings.md
+++ b/doc/development/packages/settings.md
@@ -35,6 +35,7 @@ This page includes an exhaustive list of settings related to and maintained by t
| `rubygems_max_file_size` | `plan_limits` | Maximum file size for a RubyGems package file. |
| `terraform_module_max_file_size` | `plan_limits` | Maximum file size for a Terraform package file. |
| `helm_max_file_size` | `plan_limits` | Maximum file size for a Helm package file. |
+| `helm_max_packages_count` | `application_settings` | Maximum number of Helm packages that can be listed per channel. Must be at least 1. Default is 1000. |
### Container registry
diff --git a/doc/user/packages/helm_repository/_index.md b/doc/user/packages/helm_repository/_index.md
index 2704c38016b69..34a2cd971664b 100644
--- a/doc/user/packages/helm_repository/_index.md
+++ b/doc/user/packages/helm_repository/_index.md
@@ -111,7 +111,7 @@ upload:
{{< alert type="note" >}}
-When requesting a package, GitLab considers only the 1000 most recent packages created.
+When requesting a package, GitLab considers only the most recent packages up to the configured limit (default is 1000 packages, configurable by administrators).
For each package, only the most recent package file is returned.
{{< /alert >}}
diff --git a/ee/app/views/admin/application_settings/_ee_package_registry.haml b/ee/app/views/admin/application_settings/_ee_package_registry.haml
index 96e95aa3f83e8..3fc39e544018c 100644
--- a/ee/app/views/admin/application_settings/_ee_package_registry.haml
+++ b/ee/app/views/admin/application_settings/_ee_package_registry.haml
@@ -10,7 +10,7 @@
- docs_link = link_to('', help_page_path('user/packages/package_registry/supported_functionality.md', { anchor: 'deleting-packages' }))
= safe_format(s_('PackageRegistry|There are security risks if packages are deleted while request forwarding is enabled. %{docs_link_start}What are the risks?%{docs_link_end}'), tag_pair(docs_link, :docs_link_start, :docs_link_end))
- %fieldset
+ %fieldset{ class: '!gl-mb-0' }
.form-group
= f.gitlab_ui_checkbox_component :npm_package_requests_forwarding,
s_('PackageRegistry|Forward %{package_type} package requests') % { package_type: 'npm' }
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 49c51f2324458..73c35e7d72480 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -35266,6 +35266,12 @@ msgstr ""
msgid "Maximum number of %{name} (%{count}) exceeded"
msgstr ""
+msgid "Maximum number of Helm packages per channel"
+msgstr ""
+
+msgid "Maximum number of Helm packages that can be listed per channel. Must be at least 1."
+msgstr ""
+
msgid "Maximum number of changes (branches or tags) in a single push above which a bulk push event is created (default is 3). Setting to 0 does not disable throttling."
msgstr ""
@@ -41033,6 +41039,9 @@ msgstr ""
msgid "Package forwarding"
msgstr ""
+msgid "Package limits"
+msgstr ""
+
msgid "Package name of the app in Google Play."
msgstr ""
@@ -41135,7 +41144,7 @@ msgstr ""
msgid "PackageRegistry|Configure in settings"
msgstr ""
-msgid "PackageRegistry|Configure package forwarding and package file size limits."
+msgid "PackageRegistry|Configure package forwarding, package limits, and package file size limits."
msgstr ""
msgid "PackageRegistry|Copy .pypirc content"
@@ -54923,6 +54932,9 @@ msgstr ""
msgid "Setting enforced"
msgstr ""
+msgid "Setting high package limits can impact database performance. Consider the size of your instance when configuring these values."
+msgstr ""
+
msgid "Settings"
msgstr ""
diff --git a/spec/finders/packages/helm/packages_finder_spec.rb b/spec/finders/packages/helm/packages_finder_spec.rb
index 5037a9e620581..f1f5eb16355a8 100644
--- a/spec/finders/packages/helm/packages_finder_spec.rb
+++ b/spec/finders/packages/helm/packages_finder_spec.rb
@@ -64,11 +64,26 @@
let_it_be(:helm_package3) { create(:helm_package, project: project1) }
let_it_be(:helm_package4) { create(:helm_package, project: project1) }
- before do
- stub_const("#{described_class}::MAX_PACKAGES_COUNT", 2)
- end
+ context 'with max_packages_count set to 2' do
+ before do
+ allow(::Gitlab::CurrentSettings)
+ .to receive_message_chain(:package_registry, :fetch)
+ .with('helm_max_packages_count', anything)
+ .and_return(2)
+ end
+
+ subject(:limited_packages_finder) { finder.execute }
+
+ it 'returns only 2 packages' do
+ packages = limited_packages_finder
- it { is_expected.to eq([helm_package4, helm_package3]) }
+ aggregate_failures do
+ expect(packages.size).to eq(2)
+ expect(packages).to all(be_a(Packages::Helm::Package))
+ expect(packages).to all(have_attributes(project_id: project.id))
+ end
+ end
+ end
end
end
end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 110afe6b20cf9..be7d08e9273e4 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -25,6 +25,7 @@
it { expect(setting.kroki_formats).to eq({}) }
it { expect(setting.default_branch_protection_defaults).to eq({}) }
it { expect(setting.enforce_email_subaddress_restrictions).to be(false) }
+ it { expect(setting.helm_max_packages_count).to eq(1000) }
it { expect(setting.max_decompressed_archive_size).to eq(25600) }
it { expect(setting.decompress_archive_file_timeout).to eq(210) }
it { expect(setting.bulk_import_concurrent_pipeline_batch_limit).to eq(25) }
@@ -335,6 +336,7 @@ def many_usernames(num = 100)
context 'for non-null integer attributes starting from 1' do
where(:attribute) do
%i[
+ helm_max_packages_count
bulk_import_concurrent_pipeline_batch_limit
code_suggestions_api_rate_limit
concurrent_bitbucket_import_jobs_limit
diff --git a/spec/views/admin/application_settings/_packages_limits_settings.html.haml_spec.rb b/spec/views/admin/application_settings/_packages_limits_settings.html.haml_spec.rb
new file mode 100644
index 0000000000000..2ae28041a1c66
--- /dev/null
+++ b/spec/views/admin/application_settings/_packages_limits_settings.html.haml_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'admin/application_settings/_packages_limits_settings', feature_category: :package_registry do
+ let_it_be(:application_setting) { build(:application_setting) }
+
+ before do
+ assign(:application_setting, application_setting)
+ allow(view).to receive(:form_errors).and_return('')
+ end
+
+ it 'renders package limits settings form' do
+ render
+
+ expect(rendered).to have_selector('.gl-mt-7 h4', text: 'Package limits')
+ expect(rendered).to have_selector('p',
+ text: 'Setting high package limits can impact database performance. ' \
+ 'Consider the size of your instance when configuring these values.')
+ expect(rendered).to have_selector(
+ 'form.fieldset-form[action="/admin/application_settings/ci_cd#js-package-settings"]' \
+ '[method="post"][data-testid="package-limits-form"]'
+ ) do |form|
+ expect(form).to have_field('_method', type: 'hidden', with: 'patch')
+ end
+ end
+
+ it 'renders helm packages count field with default value' do
+ render
+
+ expect(rendered).to have_selector('form') do |form|
+ expect(form).to have_selector('label', text: 'Maximum number of Helm packages per channel')
+ expect(form).to have_field(
+ 'application_setting[helm_max_packages_count]',
+ type: 'number'
+ )
+ expect(form).to have_selector(
+ "input[placeholder='#{ApplicationSetting::DEFAULT_HELM_MAX_PACKAGES_COUNT}']"
+ )
+ expect(form).to have_selector('.form-text',
+ text: 'Maximum number of Helm packages that can be listed per channel. Must be at least 1.')
+ end
+ end
+
+ it 'renders save changes button with pajamas' do
+ render
+
+ expect(rendered).to have_button('Save changes', class: ['gl-button'])
+ end
+end
--
GitLab