From e1133f7b7aa6f39f44eafc0badc67c333138c906 Mon Sep 17 00:00:00 2001
From: Rajendra Kadam <rkadam@gitlab.com>
Date: Wed, 14 Feb 2024 11:04:23 +0000
Subject: [PATCH] Move downstream pipeline rate limit to application setting
Make setting accessible from UI for changing settings
Add specs for the new application setting in API and model
Use the new setting column value in pipeline service rate limiter
Update documentation to reflect the new setting changes
This is behind the feature flag
ci_rate_limit_downstream_pipelines.
---
app/helpers/application_settings_helper.rb | 3 +-
app/models/application_setting.rb | 6 ++-
.../application_setting_implementation.rb | 3 +-
.../ci/trigger_downstream_pipeline_service.rb | 4 --
.../application_setting_rate_limits.json | 5 +++
.../application_settings/_ci_cd.html.haml | 5 +++
.../settings/continuous_integration.md | 12 ++++++
doc/api/settings.md | 4 +-
lib/api/settings.rb | 1 +
lib/gitlab/application_rate_limiter.rb | 2 +-
locale/gitlab.pot | 6 +++
spec/features/admin/admin_settings_spec.rb | 2 +
.../application_settings_helper_spec.rb | 2 +-
spec/models/application_setting_spec.rb | 2 +
spec/requests/api/settings_spec.rb | 39 ++++++++++++++++++-
...rigger_downstream_pipeline_service_spec.rb | 2 +-
16 files changed, 85 insertions(+), 13 deletions(-)
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 1affdd8f4339b..2af212fd75c80 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -514,7 +514,8 @@ def visible_attributes
:ci_max_total_yaml_size_bytes,
:project_jobs_api_rate_limit,
:security_txt_content,
- :allow_project_creation_for_guest_and_below
+ :allow_project_creation_for_guest_and_below,
+ :downstream_pipeline_trigger_limit_per_project_user_sha
].tap do |settings|
next if Gitlab.com?
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 4cb918c9c8b0d..e39dc12764149 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -597,11 +597,13 @@ def self.kroki_formats_attributes
:sidekiq_job_limiter_compression_threshold_bytes,
:sidekiq_job_limiter_limit_bytes,
:terminal_max_session_time,
- :users_get_by_id_limit
+ :users_get_by_id_limit,
+ :downstream_pipeline_trigger_limit_per_project_user_sha
end
jsonb_accessor :rate_limits,
- members_delete_limit: [:integer, { default: 60 }]
+ members_delete_limit: [:integer, { default: 60 }],
+ downstream_pipeline_trigger_limit_per_project_user_sha: [:integer, { default: 0 }]
validates :rate_limits, json_schema: { filename: "application_setting_rate_limits" }
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index e0b334780371e..c3ff9c50e7646 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -280,7 +280,8 @@ def defaults # rubocop:disable Metrics/AbcSize
security_txt_content: nil,
allow_project_creation_for_guest_and_below: true,
enable_member_promotion_management: false,
- security_approval_policies_limit: 5
+ security_approval_policies_limit: 5,
+ downstream_pipeline_trigger_limit_per_project_user_sha: 0
}.tap do |hsh|
hsh.merge!(non_production_defaults) unless Rails.env.production?
end
diff --git a/app/services/ci/trigger_downstream_pipeline_service.rb b/app/services/ci/trigger_downstream_pipeline_service.rb
index ac95c4596b137..4fc09fd749288 100644
--- a/app/services/ci/trigger_downstream_pipeline_service.rb
+++ b/app/services/ci/trigger_downstream_pipeline_service.rb
@@ -3,10 +3,6 @@
module Ci
# Enqueues the downstream pipeline worker.
class TriggerDownstreamPipelineService
- # This is a temporary constant. It may be converted into an application setting
- # in the future. See https://gitlab.com/gitlab-org/gitlab/-/issues/425941.
- DOWNSTREAM_PIPELINE_TRIGGER_LIMIT_PER_PROJECT_USER_SHA = 200
-
def initialize(bridge)
@bridge = bridge
@current_user = bridge.user
diff --git a/app/validators/json_schemas/application_setting_rate_limits.json b/app/validators/json_schemas/application_setting_rate_limits.json
index e74295291dfdc..183c632744717 100644
--- a/app/validators/json_schemas/application_setting_rate_limits.json
+++ b/app/validators/json_schemas/application_setting_rate_limits.json
@@ -8,6 +8,11 @@
"type": "integer",
"minimum": 0,
"description": "Number of project or group members a user can delete per minute."
+ },
+ "downstream_pipeline_trigger_limit_per_project_user_sha": {
+ "type": "integer",
+ "minimum": 0,
+ "description": "Maximum number of downstream pipelines triggered in a project per user"
}
}
}
diff --git a/app/views/admin/application_settings/_ci_cd.html.haml b/app/views/admin/application_settings/_ci_cd.html.haml
index f63a9862c13c5..a808c580c2fe8 100644
--- a/app/views/admin/application_settings/_ci_cd.html.haml
+++ b/app/views/admin/application_settings/_ci_cd.html.haml
@@ -50,6 +50,11 @@
= f.number_field :ci_max_includes, class: 'form-control gl-form-input'
.form-text.text-muted
= s_('AdminSettings|The maximum number of included files per pipeline.')
+ .form-group
+ = f.label :downstream_pipeline_trigger_limit_per_project_user_sha, s_('AdminSettings|Maximum downstream pipelines triggered in a project per user'), class: 'label-bold'
+ = f.number_field :downstream_pipeline_trigger_limit_per_project_user_sha, min: 0, class: 'form-control gl-form-input'
+ .form-text.text-muted
+ = s_('AdminSettings|The maximum number of downstream pipelines triggered in a project per user.')
.form-group
= f.label :ci_config_path, _('Default CI/CD configuration file'), class: 'label-bold'
= f.text_field :default_ci_config_path, class: 'form-control gl-form-input', placeholder: '.gitlab-ci.yml'
diff --git a/doc/administration/settings/continuous_integration.md b/doc/administration/settings/continuous_integration.md
index 67f5889dd61ec..d8ba43d78a29d 100644
--- a/doc/administration/settings/continuous_integration.md
+++ b/doc/administration/settings/continuous_integration.md
@@ -205,6 +205,18 @@ The default is `150`.
1. Change the value of **Maximum includes**.
1. Select **Save changes** for the changes to take effect.
+## Maximum downstream pipelines triggered per project
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144077) in GitLab 16.10 [with feature flag](../feature_flags.md) named `ci_rate_limit_downstream_pipelines`. Disabled by default.
+
+The maximum number of [downstream pipelines](../../ci/pipelines/downstream_pipelines.md) per project per user can be set at the instance level.
+The default is `0` (no restriction).
+
+1. On the left sidebar, at the bottom, select **Admin Area**.
+1. Select **Settings > CI/CD**.
+1. Change the value of **Maximum downstream pipelines triggered in a project per user**.
+1. Select **Save changes** for the changes to take effect.
+
## Default CI/CD configuration file
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/18073) in GitLab 12.5.
diff --git a/doc/api/settings.md b/doc/api/settings.md
index 5e2663e9fec07..922cb2bd3c86d 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -281,7 +281,8 @@ Example response:
"bulk_import_max_download_file_size": 5120,
"project_jobs_api_rate_limit": 600,
"security_txt_content": null,
- "bulk_import_concurrent_pipeline_batch_limit": 25
+ "bulk_import_concurrent_pipeline_batch_limit": 25,
+ "downstream_pipeline_trigger_limit_per_project_user_sha": 0
}
```
@@ -390,6 +391,7 @@ listed in the descriptions of the relevant settings.
| `domain_denylist_enabled` | boolean | no | (**If enabled, requires:** `domain_denylist`) Allows blocking sign-ups from emails from specific domains. |
| `domain_denylist` | array of strings | no | Users with email addresses that match these domains **cannot** sign up. Wildcards allowed. Use separate lines for multiple entries. For example: `domain.com`, `*.domain.com`. |
| `domain_allowlist` | array of strings | no | Force people to use only corporate emails for sign-up. Default is `null`, meaning there is no restriction. |
+| `downstream_pipeline_trigger_limit_per_project_user_sha` | integer | no | Rate limit creation of downstream pipelines. Default: `0`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144077) in GitLab 16.10 with a [flag](../administration/feature_flags.md) named `ci_rate_limit_downstream_pipelines`. Disabled by default. |
| `dsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded DSA key. Default is `0` (no restriction). `-1` disables DSA keys. |
| `ecdsa_key_restriction` | integer | no | The minimum allowed curve size (in bits) of an uploaded ECDSA key. Default is `0` (no restriction). `-1` disables ECDSA keys. |
| `ecdsa_sk_key_restriction` | integer | no | The minimum allowed curve size (in bits) of an uploaded ECDSA_SK key. Default is `0` (no restriction). `-1` disables ECDSA_SK keys. |
diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index 6ef68ccc3de63..8c542db06a74c 100644
--- a/lib/api/settings.rb
+++ b/lib/api/settings.rb
@@ -229,6 +229,7 @@ def filter_attributes_using_license(attrs)
optional :namespace_aggregation_schedule_lease_duration_in_seconds, type: Integer, desc: 'Maximum duration (in seconds) between refreshes of namespace statistics (Default: 300)'
optional :project_jobs_api_rate_limit, type: Integer, desc: 'Maximum authenticated requests to /project/:id/jobs per minute'
optional :security_txt_content, type: String, desc: 'Public security contact information made available at https://gitlab.example.com/.well-known/security.txt'
+ optional :downstream_pipeline_trigger_limit_per_project_user_sha, type: Integer, desc: 'Maximum number of downstream pipelines triggered per project'
Gitlab::SSHPublicKey.supported_types.each do |type|
optional :"#{type}_key_restriction",
diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb
index 2e992e38a44c7..e13b54e40eea4 100644
--- a/lib/gitlab/application_rate_limiter.rb
+++ b/lib/gitlab/application_rate_limiter.rb
@@ -67,7 +67,7 @@ def rate_limits # rubocop:disable Metrics/AbcSize
threshold: -> { application_settings.projects_api_rate_limit_unauthenticated }, interval: 10.minutes
},
downstream_pipeline_trigger: {
- threshold: -> { ::Ci::TriggerDownstreamPipelineService::DOWNSTREAM_PIPELINE_TRIGGER_LIMIT_PER_PROJECT_USER_SHA }, interval: 1.minute
+ threshold: -> { application_settings.downstream_pipeline_trigger_limit_per_project_user_sha }, interval: 1.minute
}
}.freeze
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 65bb8ec785da0..5b49c71adee3c 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3584,6 +3584,9 @@ msgstr ""
msgid "AdminSettings|Limit the number of namespaces and projects that can be indexed."
msgstr ""
+msgid "AdminSettings|Maximum downstream pipelines triggered in a project per user"
+msgstr ""
+
msgid "AdminSettings|Maximum duration of a session for Git operations when 2FA is enabled."
msgstr ""
@@ -3743,6 +3746,9 @@ msgstr ""
msgid "AdminSettings|The latest artifacts for all jobs in the most recent successful pipelines in each project are stored and do not expire."
msgstr ""
+msgid "AdminSettings|The maximum number of downstream pipelines triggered in a project per user."
+msgstr ""
+
msgid "AdminSettings|The maximum number of included files per pipeline."
msgstr ""
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index d1fdbfc5329f3..8db9c45dbef90 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -459,6 +459,7 @@
uncheck 'Keep the latest artifacts for all jobs in the latest successful pipelines'
uncheck 'Enable pipeline suggestion banner'
fill_in 'application_setting_ci_max_includes', with: 200
+ fill_in 'application_setting_downstream_pipeline_trigger_limit_per_project_user_sha', with: 500
click_button 'Save changes'
end
@@ -467,6 +468,7 @@
expect(current_settings.keep_latest_artifact).to be false
expect(current_settings.suggest_pipeline_enabled).to be false
expect(current_settings.ci_max_includes).to be 200
+ expect(current_settings.downstream_pipeline_trigger_limit_per_project_user_sha).to be 500
expect(page).to have_content "Application settings saved successfully"
end
diff --git a/spec/helpers/application_settings_helper_spec.rb b/spec/helpers/application_settings_helper_spec.rb
index b378437c40755..43075a75d1ca1 100644
--- a/spec/helpers/application_settings_helper_spec.rb
+++ b/spec/helpers/application_settings_helper_spec.rb
@@ -65,7 +65,7 @@
project_download_export_limit project_export_limit project_import_limit
raw_blob_request_limit group_export_limit group_download_export_limit
group_import_limit users_get_by_id_limit search_rate_limit search_rate_limit_unauthenticated
- members_delete_limit
+ members_delete_limit downstream_pipeline_trigger_limit_per_project_user_sha
])
end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 18060d29ad52c..d88415527ad42 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -29,6 +29,7 @@
it { expect(setting.bulk_import_concurrent_pipeline_batch_limit).to eq(25) }
it { expect(setting.allow_project_creation_for_guest_and_below).to eq(true) }
it { expect(setting.members_delete_limit).to eq(60) }
+ it { expect(setting.downstream_pipeline_trigger_limit_per_project_user_sha).to eq(0) }
end
describe 'validations' do
@@ -244,6 +245,7 @@ def many_usernames(num = 100)
sidekiq_job_limiter_limit_bytes
terminal_max_session_time
users_get_by_id_limit
+ downstream_pipeline_trigger_limit_per_project_user_sha
]
end
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 01aeb88a10968..018e6b9786418 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -95,6 +95,7 @@
expect(json_response['max_login_attempts']).to be_nil
expect(json_response['failed_login_attempts_unlock_period_in_minutes']).to be_nil
expect(json_response['bulk_import_concurrent_pipeline_batch_limit']).to eq(25)
+ expect(json_response['downstream_pipeline_trigger_limit_per_project_user_sha']).to eq(0)
end
end
@@ -216,7 +217,8 @@
gitlab_shell_operation_limit: 500,
namespace_aggregation_schedule_lease_duration_in_seconds: 400,
max_import_remote_file_size: 2,
- security_txt_content: nil
+ security_txt_content: nil,
+ downstream_pipeline_trigger_limit_per_project_user_sha: 300
}
expect(response).to have_gitlab_http_status(:ok)
@@ -302,6 +304,7 @@
expect(json_response['bulk_import_max_download_file_size']).to be(1)
expect(json_response['security_txt_content']).to be(nil)
expect(json_response['bulk_import_concurrent_pipeline_batch_limit']).to be(2)
+ expect(json_response['downstream_pipeline_trigger_limit_per_project_user_sha']).to be(300)
end
end
@@ -1023,6 +1026,40 @@
end
end
+ context 'with downstream_pipeline_trigger_limit_per_project_user_sha' do
+ it 'updates the settings' do
+ put api("/application/settings", admin), params: {
+ downstream_pipeline_trigger_limit_per_project_user_sha: 200
+ }
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response).to include(
+ 'downstream_pipeline_trigger_limit_per_project_user_sha' => 200
+ )
+ end
+
+ it 'allows a zero value' do
+ put api("/application/settings", admin), params: {
+ downstream_pipeline_trigger_limit_per_project_user_sha: 0
+ }
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response).to include(
+ 'downstream_pipeline_trigger_limit_per_project_user_sha' => 0
+ )
+ end
+
+ it 'does not allow a nil value' do
+ put api("/application/settings", admin), params: {
+ downstream_pipeline_trigger_limit_per_project_user_sha: nil
+ }
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ expect(json_response['message']['downstream_pipeline_trigger_limit_per_project_user_sha'])
+ .to include(a_string_matching('is not a number'))
+ end
+ end
+
context 'with housekeeping enabled' do
it 'at least one of housekeeping_incremental_repack_period or housekeeping_optimize_repository_period is required' do
put api("/application/settings", admin), params: {
diff --git a/spec/services/ci/trigger_downstream_pipeline_service_spec.rb b/spec/services/ci/trigger_downstream_pipeline_service_spec.rb
index 71d6931658925..299f765b95cd8 100644
--- a/spec/services/ci/trigger_downstream_pipeline_service_spec.rb
+++ b/spec/services/ci/trigger_downstream_pipeline_service_spec.rb
@@ -50,7 +50,7 @@
context 'when the limit is exceeded' do
before do
- stub_const("#{described_class.name}::DOWNSTREAM_PIPELINE_TRIGGER_LIMIT_PER_PROJECT_USER_SHA", 1)
+ stub_application_setting(downstream_pipeline_trigger_limit_per_project_user_sha: 1)
end
it 'drops the bridge and does not schedule the downstream pipeline worker', :aggregate_failures do
--
GitLab