diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 73d3b4df7c8328add9e7b4f1387e57e4c33ed082..cda052b3e9ce45e90322f90ccf351c3ff7c5784b 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -639,7 +639,9 @@ def visible_attributes :anonymous_searches_allowed, :git_push_pipeline_limit, :delay_user_account_self_deletion, - :resource_usage_limits + :resource_usage_limits, + :runner_jobs_request_api_limit, + :runner_jobs_endpoints_api_limit ].tap do |settings| unless Gitlab.com? settings << :deactivate_dormant_users diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 2a73de8dd290c7e0e3edb0578222f05f9b983477..c4dea91436ece09d5e77abca4272c50e7d56ee5e 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -673,6 +673,8 @@ def self.kroki_formats_attributes :projects_api_limit, :projects_api_rate_limit_unauthenticated, :raw_blob_request_limit, + :runner_jobs_request_api_limit, + :runner_jobs_endpoints_api_limit, :search_rate_limit, :search_rate_limit_unauthenticated, :sidekiq_job_limiter_compression_threshold_bytes, @@ -1156,6 +1158,8 @@ def self.rate_limits_definition group_archive_unarchive_api_limit: [:integer, { default: 60 }], project_invited_groups_api_limit: [:integer, { default: 60 }], projects_api_limit: [:integer, { default: 2000 }], + runner_jobs_request_api_limit: [:integer, { default: 2000 }], + runner_jobs_endpoints_api_limit: [:integer, { default: 200 }], throttle_authenticated_git_http_enabled: [:boolean, { default: false }], throttle_authenticated_git_http_requests_per_period: [:integer, { default: DEFAULT_AUTHENTICATED_GIT_HTTP_LIMIT }], diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 617dd771dd72bb75390e255cf8134ef63a32df9f..23c7160269064359adc07e219e89c46eac9234ff 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -322,6 +322,8 @@ def defaults # rubocop:disable Metrics/AbcSize project_api_limit: 400, project_invited_groups_api_limit: 60, projects_api_limit: 2000, + runner_jobs_request_api_limit: 2000, + runner_jobs_endpoints_api_limit: 200, user_contributed_projects_api_limit: 100, user_projects_api_limit: 300, user_starred_projects_api_limit: 100, diff --git a/app/validators/json_schemas/application_setting_rate_limits.json b/app/validators/json_schemas/application_setting_rate_limits.json index d6303858300c9b91c538348f1adf657eb07a8240..a8908d58168cf8de1caf8e6bb2ad2f80b55811cc 100644 --- a/app/validators/json_schemas/application_setting_rate_limits.json +++ b/app/validators/json_schemas/application_setting_rate_limits.json @@ -84,6 +84,16 @@ "minimum": 0, "description": "Number of requests allowed to the GET /api/v4/projects endpoint." }, + "runner_jobs_request_api_limit": { + "type": "integer", + "minimum": 0, + "description": "Number of requests allowed to the /api/v4/jobs/request endpoint." + }, + "runner_jobs_endpoints_api_limit": { + "type": "integer", + "minimum": 0, + "description": "Number of requests allowed to the /api/v4/jobs/* endpoints." + }, "throttle_authenticated_git_http_enabled": { "type": "boolean", "description": "Enable authenticated Git HTTP request rate limit" diff --git a/app/views/admin/application_settings/_runner_jobs_limits.html.haml b/app/views/admin/application_settings/_runner_jobs_limits.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..973823f8dc1b453d0c2379ab1debedd36c14ec9c --- /dev/null +++ b/app/views/admin/application_settings/_runner_jobs_limits.html.haml @@ -0,0 +1,21 @@ += gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-runner-jobs-limits-settings'), html: { class: 'fieldset-form' } do |f| + = form_errors(@application_setting) + + %fieldset + = _('Set to 0 to disable rate limits.') + + %fieldset + .form-group + = f.label :runner_jobs_request_api_limit, s_('Runners|Maximum requests per minute to /jobs/request endpoint') + = f.number_field :runner_jobs_request_api_limit, class: 'form-control gl-form-input' + .form-text.gl-text-muted + = s_('Runners|Limits how many API requests a runner can make to request jobs within a 1-minute window.') + + %fieldset + .form-group + = f.label :runner_jobs_endpoints_api_limit, s_('Runners|Maximum requests per minute to other Runner Jobs API endpoints') + = f.number_field :runner_jobs_endpoints_api_limit, class: 'form-control gl-form-input' + .form-text.gl-text-muted + = s_('Runners|Limits how many API requests a runner can make to jobs endpoints within a 1-minute window, per job token.') + + = f.submit _('Save changes'), pajamas_button: true diff --git a/app/views/admin/application_settings/_runner_registrars_form.html.haml b/app/views/admin/application_settings/_runners_settings_form.html.haml similarity index 70% rename from app/views/admin/application_settings/_runner_registrars_form.html.haml rename to app/views/admin/application_settings/_runners_settings_form.html.haml index 6d4b88dead72fb6f3c206a8147db537d9b275e27..f4d09e1fd05e6e8004ce1788391c18d1692fdbe4 100644 --- a/app/views/admin/application_settings/_runner_registrars_form.html.haml +++ b/app/views/admin/application_settings/_runners_settings_form.html.haml @@ -24,4 +24,21 @@ checked_value: type, unchecked_value: nil + %fieldset + = _('Set to 0 to disable rate limits.') + + %fieldset + .form-group + = f.label :runner_jobs_request_api_limit, s_('Runners|Maximum requests per minute to /jobs/request endpoint') + = f.number_field :runner_jobs_request_api_limit, class: 'form-control gl-form-input' + .form-text.gl-text-muted + = _('Limits how many API requests a runner can make to request jobs within a 1-minute window.') + + %fieldset + .form-group + = f.label :runner_jobs_endpoints_api_limit, s_('Runners|Maximum requests per minute to other Runner Jobs API endpoints') + = f.number_field :runner_jobs_endpoints_api_limit, class: 'form-control gl-form-input' + .form-text.gl-text-muted + = _('Limits how many API requests a runner can make to jobs endpoints within a 1-minute window, per job token.') + = f.submit _('Save changes'), pajamas_button: true diff --git a/app/views/admin/application_settings/ci_cd.html.haml b/app/views/admin/application_settings/ci_cd.html.haml index 906c9d202bb69f9f8f442940591b2a66d6aa2058..62528141882113e6d1d2ac0187edbc02fb395b74 100644 --- a/app/views/admin/application_settings/ci_cd.html.haml +++ b/app/views/admin/application_settings/ci_cd.html.haml @@ -54,9 +54,9 @@ testid: 'runner-settings', expanded: expanded_by_default?) do |c| - c.with_description do - = _('Configure runner version management and registration settings.') + = s_('Configure runner management and registration settings.') - c.with_body do - = render 'runner_registrars_form' + = render 'runners_settings_form' = render ::Layouts::SettingsBlockComponent.new(s_('CICD|Job token permissions'), id: 'js-job-token-permissions-settings', diff --git a/doc/api/openapi/openapi_v2.yaml b/doc/api/openapi/openapi_v2.yaml index d2d49d755d102ce8a4cb1639783c6b3c44b26a1c..4bf1c6d04388ad95e3f14b88a1c1acf36ae9b360 100644 --- a/doc/api/openapi/openapi_v2.yaml +++ b/doc/api/openapi/openapi_v2.yaml @@ -40624,6 +40624,8 @@ paths: description: Conflict '422': description: Runner is orphaned + '429': + description: Too Many Requests tags: - jobs operationId: postApiV4JobsRequest @@ -40655,6 +40657,8 @@ paths: description: Unknown parameters '403': description: Forbidden + '429': + description: Too Many Requests tags: - jobs operationId: putApiV4JobsId @@ -40686,6 +40690,8 @@ paths: description: Forbidden '416': description: Range not satisfiable + '429': + description: Too Many Requests tags: - jobs operationId: patchApiV4JobsIdTrace @@ -40717,6 +40723,8 @@ paths: description: Artifacts support not enabled '413': description: File too large + '429': + description: Too Many Requests tags: - jobs operationId: postApiV4JobsIdArtifactsAuthorize @@ -40750,6 +40758,8 @@ paths: description: Artifacts support not enabled '413': description: File too large + '429': + description: Too Many Requests tags: - jobs operationId: postApiV4JobsIdArtifacts @@ -40787,6 +40797,8 @@ paths: description: Forbidden '404': description: Artifact not found + '429': + description: Too Many Requests tags: - jobs operationId: getApiV4JobsIdArtifacts diff --git a/doc/api/settings.md b/doc/api/settings.md index 88c497290052a2f7fc0ab4776d91f2fbecabcf5a..f8827bcab13a414d6bb5bf5fe0397592b597b05b 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -160,6 +160,8 @@ Example response: "package_registry_allow_anyone_to_pull_option": true, "bulk_import_max_download_file_size": 5120, "project_jobs_api_rate_limit": 600, + "runner_jobs_request_api_limit": 2000, + "runner_jobs_endpoints_api_limit": 200, "security_txt_content": null, "bulk_import_concurrent_pipeline_batch_limit": 25, "concurrent_relation_batch_export_limit": 25, @@ -355,6 +357,8 @@ Example response: "jira_connect_proxy_url": "http://gitlab.example.com", "user_defaults_to_private_profile": true, "projects_api_rate_limit_unauthenticated": 400, + "runner_jobs_request_api_limit": 2000, + "runner_jobs_endpoints_api_limit": 200, "users_api_limit_followers": 100, "users_api_limit_following": 100, "users_api_limit_status": 240, @@ -683,7 +687,9 @@ to configure other related settings. These requirements are | `polling_interval_multiplier` | float | no | Interval multiplier used by endpoints that perform polling. Set to `0` to disable polling. | | `project_export_enabled` | boolean | no | Enable project export. | | `project_jobs_api_rate_limit` | integer | no | Maximum authenticated requests to `/project/:id/jobs` per minute. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129319) in GitLab 16.5. Default: 600. | -| `projects_api_rate_limit_unauthenticated` | integer | no | Max number of requests per 10 minutes per IP address for unauthenticated requests to the [list all projects API](projects.md#list-all-projects). Default: 400. To disable throttling set to 0.| +| `projects_api_rate_limit_unauthenticated` | integer | no | Max number of requests per 10 minutes per IP address for unauthenticated requests to the [list all projects API](projects.md#list-all-projects). Default: 400. To disable throttling, set to 0.| +| `runner_jobs_request_api_limit` | integer | no | Max number of requests per minute per runner token for requests to the `/jobs/request` runner jobs API endpoint. Default: 2000. To disable throttling, set to 0. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/462537) in GitLab 18.5. | +| `runner_jobs_endpoints_api_limit` | integer | no | Max number of requests per minute per job token for requests to `/jobs/*` requests to the runner jobs API endpoints. Default: 200. To disable throttling, set to 0. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/462537) in GitLab 18.5. | | `users_api_limit_following` | integer | no | Max number of requests per minute, per user or IP address. Default: 100. Set to `0` to disable limits. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181054) in GitLab 17.10. | | `users_api_limit_followers` | integer | no | Max number of requests per minute, per user or IP address. Default: 100. Set to `0` to disable limits. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181054) in GitLab 17.10. | | `users_api_limit_status` | integer | no | Max number of requests per minute, per user or IP address. Default: 240. Set to `0` to disable limits. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181054) in GitLab 17.10. | @@ -691,7 +697,7 @@ to configure other related settings. These requirements are | `users_api_limit_key` | integer | no | Max number of requests per minute, per user or IP address. Default: 120. Set to `0` to disable limits. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181054) in GitLab 17.10. | | `users_api_limit_gpg_keys` | integer | no | Max number of requests per minute, per user or IP address. Default: 120. Set to `0` to disable limits. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181054) in GitLab 17.10. | | `users_api_limit_gpg_key` | integer | no | Max number of requests per minute, per user or IP address. Default: 120. Set to `0` to disable limits. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181054) in GitLab 17.10. | -| `virtual_registries_endpoints_api_limit` | integer | no | Max number of requests on virtual registries endpoints, per IP address, per 15 seconds. Default: 1000. Set to `0` to disabled limits. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/521692) in GitLab 17.11 | +| `virtual_registries_endpoints_api_limit` | integer | no | Max number of requests on virtual registries endpoints, per IP address, per 15 seconds. Default: 1000. To disable limits, set to `0`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/521692) in GitLab 17.11. | | `prometheus_metrics_enabled` | boolean | no | Enable Prometheus metrics. | | `protected_ci_variables` | boolean | no | CI/CD variables are protected by default. | | `disable_overriding_approvers_per_merge_request` | boolean | no | Prevent editing approval rules in projects and merge requests | @@ -701,8 +707,8 @@ to configure other related settings. These requirements are | `push_event_hooks_limit` | integer | no | Maximum number of changes (branches or tags) in a single push above which webhooks and integrations are not triggered. Setting to `0` does not disable throttling. Default: `3`. | | `rate_limiting_response_text` | string | no | When rate limiting is enabled via the `throttle_*` settings, send this plain text response when a rate limit is exceeded. 'Retry later' is sent if this is blank. | | `raw_blob_request_limit` | integer | no | Maximum number of requests per minute for each raw path (default is `300`). Set to `0` to disable throttling.| -| `search_rate_limit` | integer | no | Max number of requests per minute for performing a search while authenticated. Default: 30. To disable throttling set to 0.| -| `search_rate_limit_unauthenticated` | integer | no | Max number of requests per minute for performing a search while unauthenticated. Default: 10. To disable throttling set to 0.| +| `search_rate_limit` | integer | no | Max number of requests per minute for performing a search while authenticated. Default: 30. To disable throttling, set to 0.| +| `search_rate_limit_unauthenticated` | integer | no | Max number of requests per minute for performing a search while unauthenticated. Default: 10. To disable throttling, set to 0.| | `recaptcha_enabled` | boolean | no | (**If enabled, requires**: `recaptcha_private_key` and `recaptcha_site_key`) Enable reCAPTCHA. | | `login_recaptcha_protection_enabled` | boolean | no | Enable reCAPTCHA for login. | | `recaptcha_private_key` | string | required by: `recaptcha_enabled` | Private key for reCAPTCHA. | diff --git a/doc/user/gitlab_com/_index.md b/doc/user/gitlab_com/_index.md index 6db6a93cfd7d0c96b96c1fa4894dbb5eb3236f83..c6e210182b297745be9261d90b912656e0279b23 100644 --- a/doc/user/gitlab_com/_index.md +++ b/doc/user/gitlab_com/_index.md @@ -486,6 +486,8 @@ The following table describes the rate limits for GitLab.com: | Single project requests (`/api/v4/projects/:id`) | 400 requests each minute | | Groups list requests (`/api/v4/groups`) | 200 requests each minute | | Single group requests (`/api/v4/groups/:id`) | 400 requests each minute | +| Runner jobs requests using a runner token (`/api/v4/jobs/request`) | 2,000 requests each minute | +| Runner jobs requests using a job token (`/api/v4/jobs/*`) | 200 requests each minute | More details are available on the rate limits for [protected paths](#protected-paths-throttle) and diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb index ce7c3f3bd8ee22def8be94d5e6ef31090747f5b1..3e07fdf775a6e826dd5cae35daa8c6dc5c32d281 100644 --- a/lib/api/ci/runner.rb +++ b/lib/api/ci/runner.rb @@ -143,6 +143,8 @@ class Runner < ::API::Base end resource :jobs do + helpers ::API::Helpers::RateLimiter + before { set_application_context } desc 'Request a job' do @@ -151,7 +153,8 @@ class Runner < ::API::Base [204, 'No job for Runner'], [403, 'Forbidden'], [409, 'Conflict'], - [422, 'Runner is orphaned']] + [422, 'Runner is orphaned'], + [429, 'Too Many Requests']] end params do requires :token, type: String, desc: "Runner's authentication token" @@ -191,6 +194,8 @@ class Runner < ::API::Base parser :build_json, ::Grape::Parser::Json post '/request', urgency: :low, feature_category: :continuous_integration do + check_rate_limit!(:runner_jobs_request_api, scope: [params[:token]]) + authenticate_runner!(creation_state: :finished) unless current_runner.active? @@ -230,7 +235,8 @@ class Runner < ::API::Base http_codes [[200, 'Job was updated'], [202, 'Update accepted'], [400, 'Unknown parameters'], - [403, 'Forbidden']] + [403, 'Forbidden'], + [429, 'Too Many Requests']] end params do requires :token, type: String, desc: 'Job token' @@ -245,6 +251,8 @@ class Runner < ::API::Base optional :exit_code, type: Integer, desc: "Job's exit code" end put '/:id', urgency: :low, feature_category: :continuous_integration do + check_rate_limit!(:runner_jobs_api, scope: [params[:token]]) + job = authenticate_job!(heartbeat_runner: true) Gitlab::Metrics.add_event(:update_build) @@ -266,7 +274,8 @@ class Runner < ::API::Base http_codes [[202, 'Trace was patched'], [400, 'Missing Content-Range header'], [403, 'Forbidden'], - [416, 'Range not satisfiable']] + [416, 'Range not satisfiable'], + [429, 'Too Many Requests']] end params do requires :id, type: Integer, desc: "Job's ID" @@ -274,6 +283,8 @@ class Runner < ::API::Base optional :debug_trace, type: Boolean, desc: 'Enable or Disable the debug trace' end patch '/:id/trace', urgency: :low, feature_category: :continuous_integration do + check_rate_limit!(:runner_jobs_api, scope: [params[:token]]) + job = authenticate_job!(heartbeat_runner: true) error!('400 Missing header Content-Range', 400) unless request.headers.key?('Content-Range') @@ -304,7 +315,8 @@ class Runner < ::API::Base http_codes [[200, 'Upload allowed'], [403, 'Forbidden'], [405, 'Artifacts support not enabled'], - [413, 'File too large']] + [413, 'File too large'], + [429, 'Too Many Requests']] end params do requires :id, type: Integer, desc: "Job's ID" @@ -320,6 +332,8 @@ class Runner < ::API::Base default: 'archive', values: ::Ci::JobArtifact.file_types.keys end post '/:id/artifacts/authorize', feature_category: :job_artifacts, urgency: :low do + check_rate_limit!(:runner_jobs_api, scope: [params[:token]]) + not_allowed! unless Gitlab.config.artifacts.enabled require_gitlab_workhorse! @@ -342,7 +356,8 @@ class Runner < ::API::Base [400, 'Bad request'], [403, 'Forbidden'], [405, 'Artifacts support not enabled'], - [413, 'File too large']] + [413, 'File too large'], + [429, 'Too Many Requests']] end params do requires :id, type: Integer, desc: "Job's ID" @@ -357,6 +372,8 @@ class Runner < ::API::Base optional :accessibility, type: String, desc: 'Specify accessibility level of artifact private/public' end post '/:id/artifacts', feature_category: :job_artifacts, urgency: :low do + check_rate_limit!(:runner_jobs_api, scope: [params[:token]]) + not_allowed! unless Gitlab.config.artifacts.enabled require_gitlab_workhorse! @@ -382,7 +399,8 @@ class Runner < ::API::Base [302, 'Found'], [401, 'Unauthorized'], [403, 'Forbidden'], - [404, 'Artifact not found']] + [404, 'Artifact not found'], + [429, 'Too Many Requests']] end params do requires :id, type: Integer, desc: "Job's ID" @@ -393,6 +411,8 @@ class Runner < ::API::Base route_setting :authorization, job_token_policies: :read_jobs, allow_public_access_for_enabled_project_features: [:repository, :builds] get '/:id/artifacts', feature_category: :job_artifacts do + check_rate_limit!(:runner_jobs_api, scope: [params[:token]]) + authenticate_job_via_dependent_job! authorize_job_token_policies!(current_job.project) diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index 2ee7b821c1b1c5a651108d0c5b093d7240e31aa4..e7fda01c74b0ccebbc636f338ca6fe9120d510e6 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -15,7 +15,7 @@ class << self # Application rate limits # # Threshold value can be either an Integer or a Proc - # in order to not evaluate it's value every time this method is called + # in order to not evaluate its value every time this method is called # and only do that when it's needed. def rate_limits # rubocop:disable Metrics/AbcSize { @@ -83,6 +83,8 @@ def rate_limits # rubocop:disable Metrics/AbcSize threshold: -> { application_settings.projects_api_rate_limit_unauthenticated }, interval: 10.minutes }, raw_blob: { threshold: -> { application_settings.raw_blob_request_limit }, interval: 1.minute }, + runner_jobs_request_api: { threshold: -> { application_settings.runner_jobs_request_api_limit }, interval: 1.minute }, + runner_jobs_api: { threshold: -> { application_settings.runner_jobs_endpoints_api_limit }, interval: 1.minute }, search_rate_limit: { threshold: -> { application_settings.search_rate_limit }, interval: 1.minute }, search_rate_limit_unauthenticated: { threshold: -> { application_settings.search_rate_limit_unauthenticated }, interval: 1.minute }, temporary_email_failure: { threshold: 300, interval: 1.day }, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6631589bb77eb9c71d26a81c4bd9de05a1adc998..08648f3b3771d82ed42943ba459c446445cbf2da 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -17995,7 +17995,7 @@ msgstr "" msgid "Configure repository storage." msgstr "" -msgid "Configure runner version management and registration settings." +msgid "Configure runner management and registration settings." msgstr "" msgid "Configure search access and visibility settings for search scopes." @@ -38571,6 +38571,12 @@ msgstr "" msgid "Limiting mode" msgstr "" +msgid "Limits how many API requests a runner can make to jobs endpoints within a 1-minute window, per job token." +msgstr "" + +msgid "Limits how many API requests a runner can make to request jobs within a 1-minute window." +msgstr "" + msgid "Limits how many API requests an IP address can make to artifact manager endpoints within a 15-second window." msgstr "" @@ -55324,6 +55330,12 @@ msgstr "" msgid "Runners|Learn more in the %{linkStart}Google Cloud documentation%{linkEnd}." msgstr "" +msgid "Runners|Limits how many API requests a runner can make to jobs endpoints within a 1-minute window, per job token." +msgstr "" + +msgid "Runners|Limits how many API requests a runner can make to request jobs within a 1-minute window." +msgstr "" + msgid "Runners|Loading" msgstr "" @@ -55357,6 +55369,12 @@ msgstr "" msgid "Runners|Maximum job timeout" msgstr "" +msgid "Runners|Maximum requests per minute to /jobs/request endpoint" +msgstr "" + +msgid "Runners|Maximum requests per minute to other Runner Jobs API endpoints" +msgstr "" + msgid "Runners|Median" msgstr "" diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index cec5c31ca65cf97ab74fa82ad530cbb4e6617558..8f863b7ef88a67b013888c9359e78140086f943a 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -574,10 +574,12 @@ end end - context 'Runner Registration' do - it 'allows admins to control who has access to register runners' do + context 'Runners' do + before do visit ci_cd_admin_application_settings_path + end + it 'allows admins to control who has access to register runners' do expect(current_settings.valid_runner_registrars).to eq(ApplicationSetting::VALID_RUNNER_REGISTRAR_TYPES) within_testid('runner-settings') do @@ -589,6 +591,26 @@ expect(current_settings.valid_runner_registrars).to eq([]) expect(page).to have_content 'Application settings saved successfully' end + + it 'changes `/jobs/request` rate limits settings' do + within_testid('runner-settings') do + fill_in 'Maximum requests per minute to /jobs/request endpoint', with: 0 + click_button 'Save changes' + end + + expect(page).to have_content 'Application settings saved successfully' + expect(current_settings.runner_jobs_request_api_limit).to eq(0) + end + + it 'changes Runner Jobs rate limits settings' do + within_testid('runner-settings') do + fill_in 'Maximum requests per minute to other Runner Jobs API endpoints', with: 0 + click_button 'Save changes' + end + + expect(page).to have_content 'Application settings saved successfully' + expect(current_settings.runner_jobs_endpoints_api_limit).to eq(0) + end end context 'Job token permissions' do diff --git a/spec/helpers/application_settings_helper_spec.rb b/spec/helpers/application_settings_helper_spec.rb index fe039a02868e686024c05b00d6ac0292bbdf1dc6..ef2941d828a0a78449090acaaf729496cc2ed35e 100644 --- a/spec/helpers/application_settings_helper_spec.rb +++ b/spec/helpers/application_settings_helper_spec.rb @@ -90,6 +90,8 @@ project_invited_groups_api_limit create_organization_api_limit top_level_group_creation_enabled + runner_jobs_request_api_limit + runner_jobs_endpoints_api_limit ]) end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 544709f3038065202d92c10386dbaba5b7cfb9ca..40b53c5d0fe289c866958c0ee1026875de5f4520 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -232,6 +232,8 @@ root_moved_permanently_redirection: false, ropc_without_client_credentials: true, rsa_key_restriction: 0, + runner_jobs_request_api_limit: 2000, + runner_jobs_endpoints_api_limit: 200, search_rate_limit: 30, search_rate_limit_allowlist: [], search_rate_limit_unauthenticated: 10, @@ -539,6 +541,8 @@ def many_usernames(num = 100) projects_api_limit projects_api_rate_limit_unauthenticated raw_blob_request_limit + runner_jobs_request_api_limit + runner_jobs_endpoints_api_limit search_rate_limit search_rate_limit_unauthenticated sidekiq_job_limiter_compression_threshold_bytes diff --git a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb index 4b306193e197b777d40fad81ef4d0d50264370d1..9293070773ebfa461f4e52fbcab31ceaf6c0d83c 100644 --- a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb +++ b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb @@ -11,9 +11,9 @@ let_it_be_with_reload(:group) { create(:group, parent: parent_group) } let_it_be_with_reload(:project) { create(:project, namespace: group, shared_runners_enabled: false) } - let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } - let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) } - let_it_be(:user) { create(:user, developer_of: project) } + let_it_be(:pipeline, freeze: true) { create(:ci_pipeline, project: project, ref: 'master') } + let_it_be(:runner, freeze: true) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:user, freeze: true) { create(:user, developer_of: project) } let(:job) { create(:ci_build, :pending, user: user, project: project, pipeline: pipeline, runner_id: runner.id) } let(:jwt) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } @@ -105,6 +105,12 @@ end describe 'POST /api/v4/jobs/:id/artifacts/authorize' do + it_behaves_like 'rate limited endpoint', rate_limit_key: :runner_jobs_api do + def request + authorize_artifacts_with_token_in_params(filesize: 100.megabytes.to_i) + end + end + context 'when using token as parameter' do context 'and the artifact is too large' do it_behaves_like 'rejecting artifacts that are too large' do @@ -280,6 +286,12 @@ def authorize_artifacts_with_token_in_headers(params = {}, request_headers = hea let(:request) { upload_artifacts(file_upload, headers_with_token) } end + it_behaves_like 'rate limited endpoint', rate_limit_key: :runner_jobs_api do + def request + upload_artifacts(fixture_file_upload('spec/fixtures/banana_sample.gif', 'image/gif'), headers_with_token) + end + end + it "doesn't update runner info" do expect { upload_artifacts(file_upload, headers_with_token) }.not_to change { runner.reload.contacted_at } end @@ -919,6 +931,14 @@ def expect_use_primary let(:request) { download_artifact } end + it_behaves_like 'rate limited endpoint', rate_limit_key: :runner_jobs_api do + let(:current_user) { user } + + def request + download_artifact + end + end + it "doesn't update runner info" do expect { download_artifact }.not_to change { runner.reload.contacted_at } end diff --git a/spec/requests/api/ci/runner/jobs_put_spec.rb b/spec/requests/api/ci/runner/jobs_put_spec.rb index 085a987fe93bf07fa57a64c0ed2fd37117020a3c..0e9af86a2a94607cd5f925dafdba74899de4718e 100644 --- a/spec/requests/api/ci/runner/jobs_put_spec.rb +++ b/spec/requests/api/ci/runner/jobs_put_spec.rb @@ -40,6 +40,12 @@ let(:request) { update_job(state: 'success') } end + it_behaves_like 'rate limited endpoint', rate_limit_key: :runner_jobs_api do + def request + update_job + end + end + it 'updates runner info' do expect { update_job(state: 'success') }.to change { runner.reload.contacted_at } .and change { runner_manager.reload.contacted_at } diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index a93f6f46290acaf2e46f378a98555924dda024fe..bb9fe708fc0bf4dab4112859d0ef5098fbd96dd5 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -108,6 +108,12 @@ let(:request) { post api('/jobs/request') } end + it_behaves_like 'rate limited endpoint', rate_limit_key: :runner_jobs_request_api do + def request + request_job + end + end + context 'when no token is provided' do it 'returns 400 error' do post api('/jobs/request') @@ -1436,10 +1442,10 @@ def request_job(token = runner.token, **params) expect(response).to have_gitlab_http_status(:no_content) end end + end - def request_job(token = runner.token, **params) - post api('/jobs/request'), params: params.merge(token: token) - end + def request_job(token = runner.token, **params) + post api('/jobs/request'), params: params.merge(token: token) end end end diff --git a/spec/requests/api/ci/runner/jobs_trace_spec.rb b/spec/requests/api/ci/runner/jobs_trace_spec.rb index c17029fb24a01d178ed50d333e8fc05f119c1a7d..bef02020c64a6650408fb04e8344a9470197ad3f 100644 --- a/spec/requests/api/ci/runner/jobs_trace_spec.rb +++ b/spec/requests/api/ci/runner/jobs_trace_spec.rb @@ -48,293 +48,306 @@ let(:headers_with_range) { headers.merge({ 'Content-Range' => '11-20' }) } let(:update_interval) { 10.seconds } - before do - initial_patch_the_trace - end - - it_behaves_like 'API::CI::Runner application context metadata', 'PATCH /api/:version/jobs/:id/trace' do - let(:send_request) { patch_the_trace } - end + it_behaves_like 'rate limited endpoint', rate_limit_key: :runner_jobs_api do + before do + # Disable check in Ci::Build#trigger_job_create_subscription + stub_feature_flags(ci_job_created_subscription: false) + end - it_behaves_like 'runner migrations backoff' do - let(:request) { patch_the_trace } + def request + patch api("/jobs/#{job.id}/trace"), params: ' appended', headers: headers_with_range + end end - it 'updates runner info' do - runner.update!(contacted_at: 1.year.ago) + context 'with initial patch request' do + before do + initial_patch_the_trace + end - expect { patch_the_trace }.to change { runner.reload.contacted_at } - end + it_behaves_like 'API::CI::Runner application context metadata', 'PATCH /api/:version/jobs/:id/trace' do + let(:send_request) { patch_the_trace } + end - context 'when request is valid' do - it 'gets correct response' do - expect(response).to have_gitlab_http_status(:accepted) - expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' - expect(response.header).to have_key 'Range' - expect(response.header).to have_key 'Job-Status' - expect(response.header).to have_key 'X-GitLab-Trace-Update-Interval' + it_behaves_like 'runner migrations backoff' do + let(:request) { patch_the_trace } end - context 'when job has been updated recently' do - it { expect { patch_the_trace }.not_to change { job.updated_at } } + it 'updates runner info' do + runner.update!(contacted_at: 1.year.ago) - it "changes the job's trace" do - patch_the_trace + expect { patch_the_trace }.to change { runner.reload.contacted_at } + end - expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended' + context 'when request is valid' do + it 'gets correct response' do + expect(response).to have_gitlab_http_status(:accepted) + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' + expect(response.header).to have_key 'Range' + expect(response.header).to have_key 'Job-Status' + expect(response.header).to have_key 'X-GitLab-Trace-Update-Interval' end - context 'when Runner makes a force-patch' do - it { expect { force_patch_the_trace }.not_to change { job.updated_at } } + context 'when job has been updated recently' do + it { expect { patch_the_trace }.not_to change { job.updated_at } } + + it "changes the job's trace" do + patch_the_trace + + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended' + end + + context 'when Runner makes a force-patch' do + it { expect { force_patch_the_trace }.not_to change { job.updated_at } } - it "doesn't change the build.trace" do - force_patch_the_trace + it "doesn't change the build.trace" do + force_patch_the_trace - expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' + end end end - end - context 'when job was not updated recently' do - let(:update_interval) { 16.minutes } + context 'when job was not updated recently' do + let(:update_interval) { 16.minutes } - it { expect { patch_the_trace }.to change { job.updated_at } } + it { expect { patch_the_trace }.to change { job.updated_at } } - it 'changes the job.trace' do - patch_the_trace + it 'changes the job.trace' do + patch_the_trace - expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended' - end + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended' + end - context 'when Runner makes a force-patch' do - it { expect { force_patch_the_trace }.to change { job.updated_at } } + context 'when Runner makes a force-patch' do + it { expect { force_patch_the_trace }.to change { job.updated_at } } - it "doesn't change the job.trace" do - force_patch_the_trace + it "doesn't change the job.trace" do + force_patch_the_trace - expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' + end end end - end - context 'when project for the build has been deleted' do - let(:job) do - create(:ci_build, :running, :trace_live, runner_id: runner.id, pipeline: pipeline) do |job| - job.project.update!(pending_delete: true) + context 'when project for the build has been deleted' do + let(:job) do + create(:ci_build, :running, :trace_live, runner_id: runner.id, pipeline: pipeline) do |job| + job.project.update!(pending_delete: true) + end end - end - it 'responds with forbidden' do - expect(response).to have_gitlab_http_status(:forbidden) + it 'responds with forbidden' do + expect(response).to have_gitlab_http_status(:forbidden) + end end - end - context 'when trace is patched' do - before do - patch_the_trace - end + context 'when trace is patched' do + before do + patch_the_trace + end - it 'has valid trace' do - expect(response).to have_gitlab_http_status(:accepted) - expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended' - end + it 'has valid trace' do + expect(response).to have_gitlab_http_status(:accepted) + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended' + end - context 'when canceling is supported' do - include_context 'when canceling support' + context 'when canceling is supported' do + include_context 'when canceling support' - context 'when job is cancelled' do - before do - job.cancel + context 'when job is cancelled' do + before do + job.cancel + end + + it 'patching the trace is allowed' do + patch_the_trace + + expect(response).to have_gitlab_http_status(:accepted) + end end + end - it 'patching the trace is allowed' do - patch_the_trace + context 'when canceling is not supported' do + context 'when job is canceled' do + before do + job.cancel + end - expect(response).to have_gitlab_http_status(:accepted) + it 'patching the trace returns forbidden' do + patch_the_trace + + expect(response).to have_gitlab_http_status(:forbidden) + end end end - end - context 'when canceling is not supported' do - context 'when job is canceled' do + context 'when redis data are flushed' do before do - job.cancel + redis_trace_chunks_cleanup! end - it 'patching the trace returns forbidden' do - patch_the_trace + it 'has empty trace' do + expect(job.reload.trace.raw).to eq '' + end - expect(response).to have_gitlab_http_status(:forbidden) + context 'when we perform partial patch' do + before do + patch_the_trace('hello', headers.merge({ 'Content-Range' => "28-32/5" })) + end + + it 'returns an error' do + expect(response).to have_gitlab_http_status(:range_not_satisfiable) + expect(response.header['Range']).to eq('0-0') + end + end + + context 'when we resend full trace' do + before do + patch_the_trace('BUILD TRACE appended appended hello', headers.merge('Content-Range' => "0-34/35")) + end + + it 'succeeds with updating trace' do + expect(response).to have_gitlab_http_status(:accepted) + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended hello' + end end end end - context 'when redis data are flushed' do + context 'when concurrent update of trace is happening' do before do - redis_trace_chunks_cleanup! + job.trace.write('wb') do + patch_the_trace + end end - it 'has empty trace' do - expect(job.reload.trace.raw).to eq '' + it 'returns that operation conflicts' do + expect(response).to have_gitlab_http_status(:conflict) end + end - context 'when we perform partial patch' do - before do - patch_the_trace('hello', headers.merge({ 'Content-Range' => "28-32/5" })) - end - - it 'returns an error' do - expect(response).to have_gitlab_http_status(:range_not_satisfiable) - expect(response.header['Range']).to eq('0-0') - end - end + context 'when canceling is supported' do + include_context 'when canceling support' - context 'when we resend full trace' do - before do - patch_the_trace('BUILD TRACE appended appended hello', headers.merge({ 'Content-Range' => "0-34/35" })) - end + it 'receives status in header' do + job.cancel + patch_the_trace - it 'succeeds with updating trace' do - expect(response).to have_gitlab_http_status(:accepted) - expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended hello' - end + expect(response.header['Job-Status']).to eq 'canceling' end end - end - context 'when concurrent update of trace is happening' do - before do - job.trace.write('wb') do + context 'when canceling is not supported' do + it 'receives status in header' do + job.cancel patch_the_trace + + expect(response.header['Job-Status']).to eq 'canceled' end end - it 'returns that operation conflicts' do - expect(response).to have_gitlab_http_status(:conflict) - end - end + context 'when build trace is being watched' do + before do + job.trace.being_watched! + end + + it 'returns X-GitLab-Trace-Update-Interval as 3' do + patch_the_trace - context 'when canceling is supported' do - include_context 'when canceling support' + expect(response).to have_gitlab_http_status(:accepted) + expect(response.header['X-GitLab-Trace-Update-Interval']).to eq('3') + end + end - it 'receives status in header' do - job.cancel - patch_the_trace + context 'when build trace is not being watched' do + it 'returns the interval in X-GitLab-Trace-Update-Interval' do + patch_the_trace - expect(response.header['Job-Status']).to eq 'canceling' + expect(response).to have_gitlab_http_status(:accepted) + expect(response.header['X-GitLab-Trace-Update-Interval']).to eq('60') + end end end - context 'when canceling is not supported' do - it 'receives status in header' do - job.cancel - patch_the_trace + context 'when job does not exist anymore' do + it 'returns 403 Forbidden' do + patch_the_trace(job_id: non_existing_record_id) - expect(response.header['Job-Status']).to eq 'canceled' + expect(response).to have_gitlab_http_status(:forbidden) end end - context 'when build trace is being watched' do + context 'when Runner makes a force-patch' do before do - job.trace.being_watched! + force_patch_the_trace end - it 'returns X-GitLab-Trace-Update-Interval as 3' do - patch_the_trace - + it 'gets correct response' do expect(response).to have_gitlab_http_status(:accepted) - expect(response.header['X-GitLab-Trace-Update-Interval']).to eq('3') + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' + expect(response.header).to have_key 'Range' + expect(response.header).to have_key 'Job-Status' end end - context 'when build trace is not being watched' do - it 'returns the interval in X-GitLab-Trace-Update-Interval' do - patch_the_trace + context 'when content-range start is too big' do + let(:headers_with_range) { headers.merge({ 'Content-Range' => '15-20/6' }) } - expect(response).to have_gitlab_http_status(:accepted) - expect(response.header['X-GitLab-Trace-Update-Interval']).to eq('60') + it 'gets 416 error response with range headers' do + expect(response).to have_gitlab_http_status(:range_not_satisfiable) + expect(response.header).to have_key 'Range' + expect(response.header['Range']).to eq '0-11' end end - end - - context 'when job does not exist anymore' do - it 'returns 403 Forbidden' do - patch_the_trace(job_id: non_existing_record_id) - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when Runner makes a force-patch' do - before do - force_patch_the_trace - end + context 'when content-range start is too small' do + let(:headers_with_range) { headers.merge({ 'Content-Range' => '8-20/13' }) } - it 'gets correct response' do - expect(response).to have_gitlab_http_status(:accepted) - expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' - expect(response.header).to have_key 'Range' - expect(response.header).to have_key 'Job-Status' + it 'gets 416 error response with range headers' do + expect(response).to have_gitlab_http_status(:range_not_satisfiable) + expect(response.header).to have_key 'Range' + expect(response.header['Range']).to eq '0-11' + end end - end - context 'when content-range start is too big' do - let(:headers_with_range) { headers.merge({ 'Content-Range' => '15-20/6' }) } + context 'when Content-Range header is missing' do + let(:headers_with_range) { headers } - it 'gets 416 error response with range headers' do - expect(response).to have_gitlab_http_status(:range_not_satisfiable) - expect(response.header).to have_key 'Range' - expect(response.header['Range']).to eq '0-11' + it { expect(response).to have_gitlab_http_status(:bad_request) } end - end - context 'when content-range start is too small' do - let(:headers_with_range) { headers.merge({ 'Content-Range' => '8-20/13' }) } + context 'when job has been errased' do + let(:job) { create(:ci_build, runner_id: runner.id, erased_at: Time.now) } - it 'gets 416 error response with range headers' do - expect(response).to have_gitlab_http_status(:range_not_satisfiable) - expect(response.header).to have_key 'Range' - expect(response.header['Range']).to eq '0-11' + it { expect(response).to have_gitlab_http_status(:forbidden) } end - end - - context 'when Content-Range header is missing' do - let(:headers_with_range) { headers } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end - context 'when job has been errased' do - let(:job) { create(:ci_build, runner_id: runner.id, erased_at: Time.now) } - - it { expect(response).to have_gitlab_http_status(:forbidden) } - end - - context 'when the job log is too big' do - before do - project.actual_limits.update!(ci_jobs_trace_size_limit: 1) - end + context 'when the job log is too big' do + before do + project.actual_limits.update!(ci_jobs_trace_size_limit: 1) + end - it 'returns 403 Forbidden' do - patch_the_trace(' appended', headers.merge({ 'Content-Range' => "#{1.megabyte}-#{1.megabyte + 9}" })) + it 'returns 403 Forbidden' do + patch_the_trace(' appended', headers.merge({ 'Content-Range' => "#{1.megabyte}-#{1.megabyte + 9}" })) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:forbidden) + end end - end - def patch_the_trace(content = ' appended', request_headers = nil, job_id: job.id) - unless request_headers - job.trace.read do |stream| - offset = stream.size - limit = offset + content.length - 1 - request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" }) + def patch_the_trace(content = ' appended', request_headers = nil, job_id: job.id) + unless request_headers + job.trace.read do |stream| + offset = stream.size + limit = offset + content.length - 1 + request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" }) + end end - end - travel_to(job.updated_at + update_interval) do - patch api("/jobs/#{job_id}/trace"), params: content, headers: request_headers + travel_to(job.updated_at + update_interval) do + patch api("/jobs/#{job_id}/trace"), params: content, headers: request_headers + end + job.reload end - job.reload end def initial_patch_the_trace