diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-02-07 12:12:27 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-02-07 12:12:27 +0000 |
| commit | 072dbf4b87096eb0a4f82f9e0583527ed5d8f4d0 (patch) | |
| tree | 189aaf3e33d0696a01712cbda840bd7c21ce0584 | |
| parent | a7760cee245c8cffc1b322c6031023985e764a12 (diff) | |
| download | gitlab-ce-072dbf4b87096eb0a4f82f9e0583527ed5d8f4d0.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
48 files changed, 720 insertions, 189 deletions
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 624edc15972..8848cc2c9d5 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -62,11 +62,11 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController def usage_data respond_to do |format| format.html do - usage_data_json = Gitlab::Json.pretty_generate(Gitlab::UsageData.data) + usage_data_json = Gitlab::Json.pretty_generate(Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true)) render html: Gitlab::Highlight.highlight('payload.json', usage_data_json, language: 'json') end - format.json { render json: Gitlab::UsageData.to_json } + format.json { render json: Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true).to_json } end end diff --git a/app/controllers/admin/instance_review_controller.rb b/app/controllers/admin/instance_review_controller.rb index 5567ffbdc84..cf3c2b72133 100644 --- a/app/controllers/admin/instance_review_controller.rb +++ b/app/controllers/admin/instance_review_controller.rb @@ -16,7 +16,7 @@ class Admin::InstanceReviewController < Admin::ApplicationController } if Gitlab::CurrentSettings.usage_ping_enabled? - data = ::Gitlab::UsageData.data + data = Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true) counts = data[:counts] result[:instance_review].merge!( diff --git a/app/controllers/groups/runners_controller.rb b/app/controllers/groups/runners_controller.rb index f602d02a165..d171df12eac 100644 --- a/app/controllers/groups/runners_controller.rb +++ b/app/controllers/groups/runners_controller.rb @@ -9,10 +9,8 @@ class Groups::RunnersController < Groups::ApplicationController feature_category :runner def index - ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') do - finder = Ci::RunnersFinder.new(current_user: current_user, params: { group: @group }) - @group_runners_limited_count = finder.execute.except(:limit, :offset).page.total_count_with_limit(:all, limit: 1000) - end + finder = Ci::RunnersFinder.new(current_user: current_user, params: { group: @group }) + @group_runners_limited_count = finder.execute.except(:limit, :offset).page.total_count_with_limit(:all, limit: 1000) end def runner_list_group_view_vue_ui_enabled @@ -62,11 +60,9 @@ class Groups::RunnersController < Groups::ApplicationController private def runner - ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') do - @runner ||= Ci::RunnersFinder.new(current_user: current_user, params: { group: @group }).execute - .except(:limit, :offset) - .find(params[:id]) - end + @runner ||= Ci::RunnersFinder.new(current_user: current_user, params: { group: @group }).execute + .except(:limit, :offset) + .find(params[:id]) end def runner_params diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index e125385f841..a290ef9b5e7 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -23,11 +23,6 @@ module Groups @group_runners = runners_finder.execute.page(params[:page]).per(NUMBER_OF_RUNNERS_PER_PAGE) @sort = runners_finder.sort_key - - # Allow sql generated by the two relations above, @all_group_runners and @group_runners - ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') do - render - end end def update diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb index 3ebf6bd1562..4c4bd3fc5a6 100644 --- a/app/finders/ci/runners_finder.rb +++ b/app/finders/ci/runners_finder.rb @@ -53,13 +53,7 @@ module Ci when :direct Ci::Runner.belonging_to_group(@group.id) when :descendants, nil - if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, @group, default_enabled: :yaml) - Ci::Runner.belonging_to_group_or_project_descendants(@group.id) - else - # Getting all runners from the group itself and all its descendant groups/projects - descendant_projects = Project.for_group_and_its_subgroups(@group) - Ci::Runner.legacy_belonging_to_group_or_project(@group.self_and_descendants, descendant_projects) - end + Ci::Runner.belonging_to_group_or_project_descendants(@group.id) else raise ArgumentError, 'Invalid membership filter' end diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 0f8a4be03a7..11150e839a3 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -13,7 +13,7 @@ module Ci include TaggableQueries include Presentable - add_authentication_token_field :token, encrypted: :optional + add_authentication_token_field :token, encrypted: :optional, expires_at: :compute_token_expiration, expiration_enforced?: :token_expiration_enforced? enum access_level: { not_protected: 0, @@ -119,30 +119,6 @@ module Ci .where(ci_runner_namespaces: { namespace_id: group_self_and_ancestors_ids }) } - # deprecated - # split this into: belonging_to_group & belonging_to_group_and_ancestors - scope :legacy_belonging_to_group, -> (group_id, include_ancestors: false) { - groups = ::Group.where(id: group_id) - groups = groups.self_and_ancestors if include_ancestors - - joins(:runner_namespaces) - .where(ci_runner_namespaces: { namespace_id: groups }) - .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') - } - - # deprecated - scope :legacy_belonging_to_group_or_project, -> (group_id, project_id) { - groups = ::Group.where(id: group_id) - - group_runners = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: groups }) - project_runners = joins(:runner_projects).where(ci_runner_projects: { project_id: project_id }) - - union_sql = ::Gitlab::SQL::Union.new([group_runners, project_runners]).to_sql - - from("(#{union_sql}) #{table_name}") - .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') - } - scope :belonging_to_parent_group_of_project, -> (project_id) { raise ArgumentError, "only 1 project_id allowed for performance reasons" unless project_id.is_a?(Integer) @@ -165,16 +141,9 @@ module Ci end scope :group_or_instance_wide, -> (group) do - group_and_ancestor_runners = - if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, group, default_enabled: :yaml) - belonging_to_group_and_ancestors(group.id) - else - legacy_belonging_to_group(group.id, include_ancestors: true) - end - from_union( [ - group_and_ancestor_runners, + belonging_to_group_and_ancestors(group.id), group.shared_runners ], remove_duplicates: false @@ -479,6 +448,21 @@ module Ci end end + def compute_token_expiration + case runner_type + when 'instance_type' + compute_token_expiration_instance + when 'group_type' + compute_token_expiration_group + when 'project_type' + compute_token_expiration_project + end + end + + def self.token_expiration_enforced? + Feature.enabled?(:enforce_runner_token_expires_at, default_enabled: :yaml) + end + private EXECUTOR_NAME_TO_TYPES = { @@ -498,6 +482,20 @@ module Ci EXECUTOR_TYPE_TO_NAMES = EXECUTOR_NAME_TO_TYPES.invert.freeze + def compute_token_expiration_instance + return unless expiration_interval = Gitlab::CurrentSettings.runner_token_expiration_interval + + expiration_interval.seconds.from_now + end + + def compute_token_expiration_group + ::Group.where(id: runner_namespaces.map(&:namespace_id)).map(&:effective_runner_token_expiration_interval).compact.min&.from_now + end + + def compute_token_expiration_project + Project.where(id: runner_projects.map(&:project_id)).map(&:effective_runner_token_expiration_interval).compact.min&.from_now + end + def cleanup_runner_queue Gitlab::Redis::SharedState.with do |redis| redis.del(runner_queue_key) diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb index 34c8630bb90..f44ad8ebe90 100644 --- a/app/models/concerns/token_authenticatable.rb +++ b/app/models/concerns/token_authenticatable.rb @@ -64,6 +64,18 @@ module TokenAuthenticatable mod.define_method("format_#{token_field}") do |token| token end + + mod.define_method("#{token_field}_expires_at") do + strategy.expires_at(self) + end + + mod.define_method("#{token_field}_expired?") do + strategy.expired?(self) + end + + mod.define_method("#{token_field}_with_expiration") do + strategy.token_with_expiration(self) + end end def token_authenticatable_module diff --git a/app/models/concerns/token_authenticatable_strategies/base.rb b/app/models/concerns/token_authenticatable_strategies/base.rb index f72a41f06b1..2cec4ab460e 100644 --- a/app/models/concerns/token_authenticatable_strategies/base.rb +++ b/app/models/concerns/token_authenticatable_strategies/base.rb @@ -7,6 +7,7 @@ module TokenAuthenticatableStrategies def initialize(klass, token_field, options) @klass = klass @token_field = token_field + @expires_at_field = "#{token_field}_expires_at" @options = options end @@ -44,6 +45,25 @@ module TokenAuthenticatableStrategies instance.save! if Gitlab::Database.read_write? end + def expires_at(instance) + instance.read_attribute(@expires_at_field) + end + + def expired?(instance) + return false unless expirable? && token_expiration_enforced? + + exp = expires_at(instance) + !!exp && Time.current > exp + end + + def expirable? + !!@options[:expires_at] + end + + def token_with_expiration(instance) + API::Support::TokenWithExpiration.new(self, instance) + end + def self.fabricate(model, field, options) if options[:digest] && options[:encrypted] raise ArgumentError, _('Incompatible options set!') @@ -64,6 +84,10 @@ module TokenAuthenticatableStrategies new_token = generate_available_token formatted_token = format_token(instance, new_token) set_token(instance, formatted_token) + + if expirable? + instance[@expires_at_field] = @options[:expires_at].to_proc.call(instance) + end end def unique @@ -82,11 +106,21 @@ module TokenAuthenticatableStrategies end def relation(unscoped) - unscoped ? @klass.unscoped : @klass + unscoped ? @klass.unscoped : @klass.where(not_expired) end def token_set?(instance) raise NotImplementedError end + + def token_expiration_enforced? + return true unless @options[:expiration_enforced?] + + @options[:expiration_enforced?].to_proc.call(@klass) + end + + def not_expired + Arel.sql("#{@expires_at_field} IS NULL OR #{@expires_at_field} >= NOW()") if expirable? && token_expiration_enforced? + end end end diff --git a/app/services/service_ping/build_payload_service.rb b/app/services/service_ping/build_payload_service.rb index 2bef3d32103..102d8ccac5b 100644 --- a/app/services/service_ping/build_payload_service.rb +++ b/app/services/service_ping/build_payload_service.rb @@ -19,7 +19,7 @@ module ServicePing end def raw_payload - @raw_payload ||= ::Gitlab::UsageData.data(force_refresh: true) + @raw_payload ||= ::Gitlab::Usage::ServicePingReport.for(mode: :values) end end end diff --git a/app/services/service_ping/submit_service.rb b/app/services/service_ping/submit_service.rb index 3e8c1848f88..6c0ca40a542 100644 --- a/app/services/service_ping/submit_service.rb +++ b/app/services/service_ping/submit_service.rb @@ -33,7 +33,7 @@ module ServicePing } submit_payload({ error: error_payload }, url: error_url) - usage_data = Gitlab::UsageData.data(force_refresh: true) + usage_data = Gitlab::Usage::ServicePingReport.for(mode: :values) response = submit_usage_data_payload(usage_data) end diff --git a/app/views/projects/_merge_request_merge_method_settings.html.haml b/app/views/projects/_merge_request_merge_method_settings.html.haml index b0e3bda2b4f..778586a592e 100644 --- a/app/views/projects/_merge_request_merge_method_settings.html.haml +++ b/app/views/projects/_merge_request_merge_method_settings.html.haml @@ -2,7 +2,9 @@ .form-group %b= s_('ProjectSettings|Merge method') - %p.text-secondary= s_('ProjectSettings|Determine what happens to the commit history when you merge a merge request.') + %p.text-secondary + = s_('ProjectSettings|Determine what happens to the commit history when you merge a merge request.') + = link_to s_('ProjectSettings|Learn about commit history.'), help_page_path('user/project/merge_requests/commits.md'), target: '_blank', rel: 'noopener noreferrer' .form-check.mb-2 = form.radio_button :merge_method, :merge, class: "js-merge-method-radio form-check-input" = label_tag :project_merge_method_merge, class: 'form-check-label' do @@ -33,4 +35,4 @@ = s_('ProjectSettings|When there is a merge conflict, the user is given the option to rebase.') %div = s_('ProjectSettings|If merge trains are enabled, merging is only possible if the branch can be rebased without conflicts.') - = link_to sprite_icon('question-o'), help_page_path('ci/pipelines/merge_trains.md', anchor: 'enable-merge-trains'), target: '_blank', rel: 'noopener noreferrer' + = link_to s_('ProjectSettings|What are merge trains?'), help_page_path('ci/pipelines/merge_trains.md', anchor: 'enable-merge-trains'), target: '_blank', rel: 'noopener noreferrer' diff --git a/app/views/shared/deploy_tokens/_form.html.haml b/app/views/shared/deploy_tokens/_form.html.haml index 902a0cad483..7289121d9eb 100644 --- a/app/views/shared/deploy_tokens/_form.html.haml +++ b/app/views/shared/deploy_tokens/_form.html.haml @@ -38,7 +38,7 @@ %fieldset.form-group.form-check = f.check_box :write_registry, class: 'form-check-input', data: { qa_selector: 'deploy_token_write_registry_checkbox' } = f.label :write_registry, 'write_registry', class: 'label-bold form-check-label' - .text-secondary= s_('DeployTokens|Allows read and write access to registry images.') + .text-secondary= s_('DeployTokens|Allows write access to registry images.') - if packages_registry_enabled?(group_or_project) %fieldset.form-group.form-check diff --git a/config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml b/config/feature_flags/development/enforce_runner_token_expires_at.yml index 337e6b11408..a1cb3bdcfdd 100644 --- a/config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml +++ b/config/feature_flags/development/enforce_runner_token_expires_at.yml @@ -1,8 +1,8 @@ --- -name: ci_find_runners_by_ci_mirrors -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74900 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/347226 -milestone: '14.7' +name: enforce_runner_token_expires_at +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78557 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/352008 +milestone: '14.8' type: development group: group::runner default_enabled: false diff --git a/db/migrate/20220203123333_add_batched_migration_max_batch.rb b/db/migrate/20220203123333_add_batched_migration_max_batch.rb new file mode 100644 index 00000000000..d16c6dd4110 --- /dev/null +++ b/db/migrate/20220203123333_add_batched_migration_max_batch.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddBatchedMigrationMaxBatch < Gitlab::Database::Migration[1.0] + def change + add_column :batched_background_migrations, :max_batch_size, :integer + end +end diff --git a/db/schema_migrations/20220203123333 b/db/schema_migrations/20220203123333 new file mode 100644 index 00000000000..9cc146c4ed7 --- /dev/null +++ b/db/schema_migrations/20220203123333 @@ -0,0 +1 @@ +87cccb30bb6f27a1acb0dd0cb905040e2a0291cefc5f26bb9a08c8be18034ca3
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 6dc825fdedb..9c95424bae7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11075,6 +11075,7 @@ CREATE TABLE batched_background_migrations ( job_arguments jsonb DEFAULT '"[]"'::jsonb NOT NULL, total_tuple_count bigint, pause_ms integer DEFAULT 100 NOT NULL, + max_batch_size integer, CONSTRAINT check_5bb0382d6f CHECK ((char_length(column_name) <= 63)), CONSTRAINT check_6b6a06254a CHECK ((char_length(table_name) <= 63)), CONSTRAINT check_batch_size_in_range CHECK ((batch_size >= sub_batch_size)), diff --git a/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md b/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md index aa0c27327d8..0a54c5d5901 100644 --- a/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md +++ b/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md @@ -1324,7 +1324,7 @@ has more information about Service Ping. ### Generate or get the cached Service Ping ```ruby -Gitlab::UsageData.to_json +Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true) ``` ### Generate a fresh new Service Ping @@ -1332,7 +1332,7 @@ Gitlab::UsageData.to_json This also refreshes the cached Service Ping displayed in the Admin Area ```ruby -Gitlab::UsageData.to_json(force_refresh: true) +Gitlab::Usage::ServicePingReport.for(mode: :values) ``` ### Generate and print diff --git a/doc/ci/environments/img/environments_open_live_environment_v14_8.png b/doc/ci/environments/img/environments_open_live_environment_v14_8.png Binary files differnew file mode 100644 index 00000000000..daf531c83c4 --- /dev/null +++ b/doc/ci/environments/img/environments_open_live_environment_v14_8.png diff --git a/doc/ci/environments/index.md b/doc/ci/environments/index.md index 90b7ff030aa..63bdd279927 100644 --- a/doc/ci/environments/index.md +++ b/doc/ci/environments/index.md @@ -380,7 +380,7 @@ places in GitLab: - In a merge request as a link:  - In the Environments view as a button: -  +  - In the Deployments view as a button:  diff --git a/doc/ci/img/environments_available_13_10.png b/doc/ci/img/environments_available_13_10.png Binary files differdeleted file mode 100644 index 94ffb0032fa..00000000000 --- a/doc/ci/img/environments_available_13_10.png +++ /dev/null diff --git a/doc/development/service_ping/index.md b/doc/development/service_ping/index.md index 9786b1a8347..02b5b81e100 100644 --- a/doc/development/service_ping/index.md +++ b/doc/development/service_ping/index.md @@ -198,9 +198,9 @@ sequenceDiagram ## How Service Ping works 1. The Service Ping [cron job](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/workers/gitlab_service_ping_worker.rb#L24) is set in Sidekiq to run weekly. -1. When the cron job runs, it calls [`Gitlab::UsageData.to_json`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb#L49). -1. `Gitlab::UsageData.to_json` [cascades down](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb) to ~400+ other counter method calls. -1. The response of all methods calls are [merged together](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb#L68) into a single JSON payload in `Gitlab::UsageData.to_json`. +1. When the cron job runs, it calls [`Gitlab::Usage::ServicePingReport.for(mode: :values)`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb). +1. `Gitlab::Usage::ServicePingReport.for(mode: :values)` [cascades down](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb) to ~400+ other counter method calls. +1. The response of all methods calls are [merged together](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb#L68) into a single JSON payload. 1. The JSON payload is then [posted to the Versions application](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb#L20) If a firewall exception is needed, the required URL depends on several things. If the hostname is `version.gitlab.com`, the protocol is `TCP`, and the port number is `443`, diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index 5598a935f77..4b63d423480 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -944,7 +944,7 @@ First ensure your backup tar file is in the backup directory described in the ```shell sudo cp 11493107454_2018_04_25_10.6.4-ce_gitlab_backup.tar /var/opt/gitlab/backups/ -sudo chown git.git /var/opt/gitlab/backups/11493107454_2018_04_25_10.6.4-ce_gitlab_backup.tar +sudo chown git:git /var/opt/gitlab/backups/11493107454_2018_04_25_10.6.4-ce_gitlab_backup.tar ``` Stop the processes that are connected to the database. Leave the rest of GitLab diff --git a/lib/api/support/token_with_expiration.rb b/lib/api/support/token_with_expiration.rb new file mode 100644 index 00000000000..2cbd562c608 --- /dev/null +++ b/lib/api/support/token_with_expiration.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module API + module Support + class TokenWithExpiration + def initialize(strategy, instance) + @strategy = strategy + @instance = instance + end + + def token + @strategy.get_token(@instance) + end + + def token_expires_at + @strategy.expires_at(@instance) + end + + def expirable? + @strategy.expirable? + end + end + end +end diff --git a/lib/api/usage_data_non_sql_metrics.rb b/lib/api/usage_data_non_sql_metrics.rb index 36325c3b742..df1144ee7d3 100644 --- a/lib/api/usage_data_non_sql_metrics.rb +++ b/lib/api/usage_data_non_sql_metrics.rb @@ -18,7 +18,7 @@ module API get 'non_sql_metrics' do Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/325534') - data = Gitlab::UsageDataNonSqlMetrics.data(force_refresh: true) + data = Gitlab::UsageDataNonSqlMetrics.data present data end diff --git a/lib/api/usage_data_queries.rb b/lib/api/usage_data_queries.rb index 1ce8feac4ab..c252c3a27c8 100644 --- a/lib/api/usage_data_queries.rb +++ b/lib/api/usage_data_queries.rb @@ -18,7 +18,7 @@ module API get 'queries' do Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/325534') - queries = Gitlab::UsageDataQueries.data(force_refresh: true) + queries = Gitlab::UsageDataQueries.data present queries end diff --git a/lib/gitlab/database/background_migration/batch_optimizer.rb b/lib/gitlab/database/background_migration/batch_optimizer.rb index 0668490dda8..58c4a214077 100644 --- a/lib/gitlab/database/background_migration/batch_optimizer.rb +++ b/lib/gitlab/database/background_migration/batch_optimizer.rb @@ -20,7 +20,8 @@ module Gitlab TARGET_EFFICIENCY = (0.9..0.95).freeze # Lower and upper bound for the batch size - ALLOWED_BATCH_SIZE = (1_000..2_000_000).freeze + MIN_BATCH_SIZE = 1_000 + MAX_BATCH_SIZE = 2_000_000 # Limit for the multiplier of the batch size MAX_MULTIPLIER = 1.2 @@ -43,7 +44,8 @@ module Gitlab return unless Feature.enabled?(:optimize_batched_migrations, type: :ops, default_enabled: :yaml) if multiplier = batch_size_multiplier - migration.batch_size = (migration.batch_size * multiplier).to_i.clamp(ALLOWED_BATCH_SIZE) + max_batch = migration.max_batch_size || MAX_BATCH_SIZE + migration.batch_size = (migration.batch_size * multiplier).to_i.clamp(MIN_BATCH_SIZE, max_batch) migration.save! end end diff --git a/lib/gitlab/database/migrations/batched_background_migration_helpers.rb b/lib/gitlab/database/migrations/batched_background_migration_helpers.rb index dcaf7fad05f..a2a4a37ab87 100644 --- a/lib/gitlab/database/migrations/batched_background_migration_helpers.rb +++ b/lib/gitlab/database/migrations/batched_background_migration_helpers.rb @@ -66,6 +66,7 @@ module Gitlab batch_max_value: nil, batch_class_name: BATCH_CLASS_NAME, batch_size: BATCH_SIZE, + max_batch_size: nil, sub_batch_size: SUB_BATCH_SIZE ) @@ -86,7 +87,7 @@ module Gitlab migration_status = batch_max_value.nil? ? :finished : :active batch_max_value ||= batch_min_value - migration = Gitlab::Database::BackgroundMigration::BatchedMigration.create!( + migration = Gitlab::Database::BackgroundMigration::BatchedMigration.new( job_class_name: job_class_name, table_name: batch_table_name, column_name: batch_column_name, @@ -97,19 +98,28 @@ module Gitlab batch_class_name: batch_class_name, batch_size: batch_size, sub_batch_size: sub_batch_size, - status: migration_status) + status: migration_status + ) - # This guard is necessary since #total_tuple_count was only introduced schema-wise, - # after this migration helper had been used for the first time. - return migration unless migration.respond_to?(:total_tuple_count) + # Below `BatchedMigration` attributes were introduced after the + # initial `batched_background_migrations` table was created, so any + # migrations that ran relying on initial table schema would not know + # about columns introduced later on because this model is not + # isolated in migrations, which is why we need to check for existence + # of these columns first. + if migration.respond_to?(:max_batch_size) + migration.max_batch_size = max_batch_size + end - # We keep track of the estimated number of tuples to reason later - # about the overall progress of a migration. - migration.total_tuple_count = Gitlab::Database::SharedModel.using_connection(connection) do - Gitlab::Database::PgClass.for_table(batch_table_name)&.cardinality_estimate + if migration.respond_to?(:total_tuple_count) + # We keep track of the estimated number of tuples to reason later + # about the overall progress of a migration. + migration.total_tuple_count = Gitlab::Database::SharedModel.using_connection(connection) do + Gitlab::Database::PgClass.for_table(batch_table_name)&.cardinality_estimate + end end - migration.save! + migration.save! migration end end diff --git a/lib/gitlab/usage/service_ping_report.rb b/lib/gitlab/usage/service_ping_report.rb new file mode 100644 index 00000000000..6cc1ee90559 --- /dev/null +++ b/lib/gitlab/usage/service_ping_report.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Gitlab + module Usage + class ServicePingReport + class << self + def for(mode:, cached: false) + case mode.to_sym + when :values + usage_data(cached) + end + end + + private + + def usage_data(cached) + Rails.cache.fetch('usage_data', force: !cached, expires_in: 2.weeks) do + Gitlab::UsageData.data + end + end + end + end + end +end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 83c26ee9ade..5af35b1f206 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -41,14 +41,8 @@ module Gitlab include Gitlab::Utils::StrongMemoize include Gitlab::Usage::TimeFrame - def data(force_refresh: false) - Rails.cache.fetch('usage_data', force: force_refresh, expires_in: 2.weeks) do - uncached_data - end - end - - def to_json(force_refresh: false) - data(force_refresh: force_refresh).to_json + def data + uncached_data end def license_usage_data diff --git a/lib/tasks/gitlab/usage_data.rake b/lib/tasks/gitlab/usage_data.rake index dc1d7cf4cdc..597773c3928 100644 --- a/lib/tasks/gitlab/usage_data.rake +++ b/lib/tasks/gitlab/usage_data.rake @@ -4,17 +4,17 @@ namespace :gitlab do namespace :usage_data do desc 'GitLab | UsageData | Generate raw SQLs for usage ping in YAML' task dump_sql_in_yaml: :environment do - puts Gitlab::UsageDataQueries.data(force_refresh: true).to_yaml + puts Gitlab::UsageDataQueries.data.to_yaml end desc 'GitLab | UsageData | Generate raw SQLs for usage ping in JSON' task dump_sql_in_json: :environment do - puts Gitlab::Json.pretty_generate(Gitlab::UsageDataQueries.data(force_refresh: true)) + puts Gitlab::Json.pretty_generate(Gitlab::UsageDataQueries.data) end desc 'GitLab | UsageData | Generate usage ping in JSON' task generate: :environment do - puts Gitlab::Json.pretty_generate(Gitlab::UsageData.data(force: true)) + puts Gitlab::Json.pretty_generate(Gitlab::Usage::ServicePingReport.for(mode: :values)) end desc 'GitLab | UsageData | Generate usage ping and send it to Versions Application' diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 279a76bbfdb..0d87a7df42b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11994,9 +11994,6 @@ msgstr "" msgid "DeployTokens|Active Deploy Tokens (%{active_tokens})" msgstr "" -msgid "DeployTokens|Allows read and write access to registry images." -msgstr "" - msgid "DeployTokens|Allows read and write access to the package registry." msgstr "" @@ -12009,6 +12006,9 @@ msgstr "" msgid "DeployTokens|Allows read-only access to the repository." msgstr "" +msgid "DeployTokens|Allows write access to registry images." +msgstr "" + msgid "DeployTokens|Copy deploy token" msgstr "" @@ -28318,6 +28318,9 @@ msgstr "" msgid "ProjectSettings|LFS objects from this repository are available to forks. %{linkStart}How do I remove them?%{linkEnd}" msgstr "" +msgid "ProjectSettings|Learn about commit history." +msgstr "" + msgid "ProjectSettings|Manage who can see the project in the public access directory." msgstr "" @@ -28519,6 +28522,9 @@ msgstr "" msgid "ProjectSettings|What are badges?" msgstr "" +msgid "ProjectSettings|What are merge trains?" +msgstr "" + msgid "ProjectSettings|When merge request pipelines are enabled in the CI/CD configuration file, pipelines validate the combined results of the source and target branches. %{link_start}How to configure merge request pipelines?%{link_end}" msgstr "" diff --git a/rubocop/cop/migration/schedule_async.rb b/rubocop/cop/migration/schedule_async.rb index f31bfa46aa7..4fdedecdf64 100644 --- a/rubocop/cop/migration/schedule_async.rb +++ b/rubocop/cop/migration/schedule_async.rb @@ -16,14 +16,23 @@ module RuboCop MSG def_node_matcher :calls_background_migration_worker?, <<~PATTERN - (send (const nil? :BackgroundMigrationWorker) {:perform_async :perform_in :bulk_perform_async :bulk_perform_in} ... ) + (send (const {cbase nil?} :BackgroundMigrationWorker) #perform_method? ...) + PATTERN + + def_node_matcher :calls_ci_database_worker?, <<~PATTERN + (send (const {(const {cbase nil?} :BackgroundMigration) nil?} :CiDatabaseWorker) #perform_method? ...) + PATTERN + + def_node_matcher :perform_method?, <<~PATTERN + {:perform_async :bulk_perform_async :perform_in :bulk_perform_in} PATTERN def on_send(node) return unless in_migration?(node) return if version(node) < ENFORCED_SINCE + return unless calls_background_migration_worker?(node) || calls_ci_database_worker?(node) - add_offense(node, location: :expression) if calls_background_migration_worker?(node) + add_offense(node, location: :expression) end end end diff --git a/rubocop/cop/scalability/bulk_perform_with_context.rb b/rubocop/cop/scalability/bulk_perform_with_context.rb index 3c5d7f39680..b96aa35bfee 100644 --- a/rubocop/cop/scalability/bulk_perform_with_context.rb +++ b/rubocop/cop/scalability/bulk_perform_with_context.rb @@ -23,6 +23,8 @@ module RuboCop Read more about it https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#worker-context MSG + BACKGROUND_MIGRATION_WORKER_NAMES = %w[BackgroundMigrationWorker CiDatabaseWorker].freeze + def_node_matcher :schedules_in_batch_without_context?, <<~PATTERN (send (...) {:bulk_perform_async :bulk_perform_in} _*) PATTERN @@ -30,7 +32,7 @@ module RuboCop def on_send(node) return if in_migration?(node) || in_spec?(node) return unless schedules_in_batch_without_context?(node) - return if name_of_receiver(node) == "BackgroundMigrationWorker" + return if scheduled_for_background_migration?(node) add_offense(node, location: :expression) end @@ -40,6 +42,10 @@ module RuboCop def in_spec?(node) file_path_for_node(node).end_with?("_spec.rb") end + + def scheduled_for_background_migration?(node) + BACKGROUND_MIGRATION_WORKER_NAMES.include?(name_of_receiver(node)) + end end end end diff --git a/spec/controllers/admin/instance_review_controller_spec.rb b/spec/controllers/admin/instance_review_controller_spec.rb index 2169be4e70c..56c8c5b0cb6 100644 --- a/spec/controllers/admin/instance_review_controller_spec.rb +++ b/spec/controllers/admin/instance_review_controller_spec.rb @@ -23,7 +23,7 @@ RSpec.describe Admin::InstanceReviewController do stub_application_setting(usage_ping_enabled: true) stub_usage_data_connections stub_database_flavor_check - ::Gitlab::UsageData.data(force_refresh: true) + ::Gitlab::Usage::ServicePingReport.for(mode: :values) subject end diff --git a/spec/features/boards/board_filters_spec.rb b/spec/features/boards/board_filters_spec.rb index 783b701991b..e37bf515088 100644 --- a/spec/features/boards/board_filters_spec.rb +++ b/spec/features/boards/board_filters_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Issue board filters', :js do let_it_be(:issue_2) { create(:labeled_issue, project: project, milestone: milestone_2, assignees: [user], labels: [project_label], confidential: true) } let_it_be(:award_emoji1) { create(:award_emoji, name: 'thumbsup', user: user, awardable: issue_1) } - let(:filtered_search) { find('[data-testid="issue_1-board-filtered-search"]') } + let(:filtered_search) { find('[data-testid="issue-board-filtered-search"]') } let(:filter_input) { find('.gl-filtered-search-term-input')} let(:filter_dropdown) { find('.gl-filtered-search-suggestion-list') } let(:filter_first_suggestion) { find('.gl-filtered-search-suggestion-list').first('.gl-filtered-search-suggestion') } diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb index dac244e4300..7e3c1abd6d1 100644 --- a/spec/finders/ci/runners_finder_spec.rb +++ b/spec/finders/ci/runners_finder_spec.rb @@ -206,7 +206,7 @@ RSpec.describe Ci::RunnersFinder do sub_group_4.runners << runner_sub_group_4 end - shared_examples '#execute' do + describe '#execute' do subject { described_class.new(current_user: user, params: params).execute } shared_examples 'membership equal to :descendants' do @@ -349,16 +349,6 @@ RSpec.describe Ci::RunnersFinder do end end - it_behaves_like '#execute' - - context 'when the FF ci_find_runners_by_ci_mirrors is disabled' do - before do - stub_feature_flags(ci_find_runners_by_ci_mirrors: false) - end - - it_behaves_like '#execute' - end - describe '#sort_key' do subject { described_class.new(current_user: user, params: params.merge(group: group)).sort_key } diff --git a/spec/lib/gitlab/database/background_migration/batch_optimizer_spec.rb b/spec/lib/gitlab/database/background_migration/batch_optimizer_spec.rb index 95863ce3765..c367f4a4493 100644 --- a/spec/lib/gitlab/database/background_migration/batch_optimizer_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batch_optimizer_spec.rb @@ -6,7 +6,11 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchOptimizer do describe '#optimize' do subject { described_class.new(migration, number_of_jobs: number_of_jobs, ema_alpha: ema_alpha).optimize! } - let(:migration) { create(:batched_background_migration, batch_size: batch_size, sub_batch_size: 100, interval: 120) } + let(:migration_params) { {} } + let(:migration) do + params = { batch_size: batch_size, sub_batch_size: 100, interval: 120 }.merge(migration_params) + create(:batched_background_migration, params) + end let(:batch_size) { 10_000 } @@ -87,6 +91,17 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchOptimizer do expect { subject }.to change { migration.reload.batch_size }.to(2_000_000) end + + context 'when max_batch_size is set' do + let(:max_batch_size) { 10000 } + let(:migration_params) { { max_batch_size: max_batch_size } } + + it 'caps the batch size at max_batch_size' do + mock_efficiency(0.7) + + expect { subject }.to change { migration.reload.batch_size }.to(max_batch_size) + end + end end context 'reaching the lower limit for the batch size' do diff --git a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb index c45149d67bf..37efff165c7 100644 --- a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb @@ -59,6 +59,7 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d batch_max_value: 1000, batch_class_name: 'MyBatchClass', batch_size: 100, + max_batch_size: 10000, sub_batch_size: 10) end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) @@ -71,6 +72,7 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d max_value: 1000, batch_class_name: 'MyBatchClass', batch_size: 100, + max_batch_size: 10000, sub_batch_size: 10, job_arguments: %w[], status: 'active', diff --git a/spec/lib/gitlab/usage/service_ping_report_spec.rb b/spec/lib/gitlab/usage/service_ping_report_spec.rb new file mode 100644 index 00000000000..e8a06883bd1 --- /dev/null +++ b/spec/lib/gitlab/usage/service_ping_report_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_caching do + let(:usage_data) { { uuid: "1111" } } + + context 'for mode: :values' do + it 'generates the service ping' do + expect(Gitlab::UsageData).to receive(:data) + + described_class.for(mode: :values) + end + end + + context 'when using cached' do + context 'for cached: true' do + let(:new_usage_data) { { uuid: "1112" } } + + it 'caches the values' do + allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data) + + expect(described_class.for(mode: :values)).to eq(usage_data) + expect(described_class.for(mode: :values, cached: true)).to eq(usage_data) + + expect(Rails.cache.fetch('usage_data')).to eq(usage_data) + end + + it 'writes to cache and returns fresh data' do + allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data) + + expect(described_class.for(mode: :values)).to eq(usage_data) + expect(described_class.for(mode: :values)).to eq(new_usage_data) + expect(described_class.for(mode: :values, cached: true)).to eq(new_usage_data) + + expect(Rails.cache.fetch('usage_data')).to eq(new_usage_data) + end + end + + context 'when no caching' do + let(:new_usage_data) { { uuid: "1112" } } + + it 'returns fresh data' do + allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data) + + expect(described_class.for(mode: :values)).to eq(usage_data) + expect(described_class.for(mode: :values)).to eq(new_usage_data) + + expect(Rails.cache.fetch('usage_data')).to eq(new_usage_data) + end + end + end +end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 3dc38b5bc1c..bea07dd9c43 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end describe '.data' do - subject { described_class.data(force_refresh: true) } + subject { described_class.data } it 'includes basic top and second level keys' do is_expected.to include(:counts) diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 08bbdfd8185..eb29db697a5 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Ci::Runner do + include StubGitlabCalls + it_behaves_like 'having unique enum values' it_behaves_like 'it has loose foreign keys' do @@ -1301,9 +1303,11 @@ RSpec.describe Ci::Runner do end it 'supports ordering by the token expiration' do - runner1 = create(:ci_runner, token_expires_at: 1.year.from_now) + runner1 = create(:ci_runner) + runner1.update!(token_expires_at: 1.year.from_now) runner2 = create(:ci_runner) - runner3 = create(:ci_runner, token_expires_at: 1.month.from_now) + runner3 = create(:ci_runner) + runner3.update!(token_expires_at: 1.month.from_now) runners = described_class.order_by('token_expires_at_asc') expect(runners).to eq([runner3, runner1, runner2]) @@ -1438,47 +1442,6 @@ RSpec.describe Ci::Runner do it { is_expected.to eq(contacted_at_stored) } end - describe '.legacy_belonging_to_group' do - shared_examples 'returns group runners' do - it 'returns the specific group runner' do - group = create(:group) - runner = create(:ci_runner, :group, groups: [group]) - unrelated_group = create(:group) - create(:ci_runner, :group, groups: [unrelated_group]) - - expect(described_class.legacy_belonging_to_group(group.id)).to contain_exactly(runner) - end - - context 'runner belonging to parent group' do - let_it_be(:parent_group) { create(:group) } - let_it_be(:parent_runner) { create(:ci_runner, :group, groups: [parent_group]) } - let_it_be(:group) { create(:group, parent: parent_group) } - - context 'when include_parent option is passed' do - it 'returns the group runner from the parent group' do - expect(described_class.legacy_belonging_to_group(group.id, include_ancestors: true)).to contain_exactly(parent_runner) - end - end - - context 'when include_parent option is not passed' do - it 'does not return the group runner from the parent group' do - expect(described_class.legacy_belonging_to_group(group.id)).to be_empty - end - end - end - end - - it_behaves_like 'returns group runners' - - context 'when feature flag :linear_runner_ancestor_scopes is disabled' do - before do - stub_feature_flags(linear_runner_ancestor_scopes: false) - end - - it_behaves_like 'returns group runners' - end - end - describe '.belonging_to_group' do it 'returns the specific group runner' do group = create(:group) @@ -1522,4 +1485,182 @@ RSpec.describe Ci::Runner do ) end end + + describe '#token_expires_at', :freeze_time do + shared_examples 'expiring token' do |interval:| + it 'expires' do + expect(runner.token_expires_at).to eq(interval.from_now) + end + end + + shared_examples 'non-expiring token' do + it 'does not expire' do + expect(runner.token_expires_at).to be_nil + end + end + + context 'no expiration' do + let(:runner) { create(:ci_runner) } + + it_behaves_like 'non-expiring token' + end + + context 'system-wide shared expiration' do + before do + stub_application_setting(runner_token_expiration_interval: 5.days.to_i) + end + + let(:runner) { create(:ci_runner) } + + it_behaves_like 'expiring token', interval: 5.days + end + + context 'system-wide group expiration' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let(:runner) { create(:ci_runner) } + + it_behaves_like 'non-expiring token' + end + + context 'system-wide project expiration' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let(:runner) { create(:ci_runner) } + + it_behaves_like 'non-expiring token' + end + + context 'group expiration' do + let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 6.days.to_i) } + let(:group) { create(:group, namespace_settings: group_settings) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } + + it_behaves_like 'expiring token', interval: 6.days + end + + context 'human-readable group expiration' do + let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval_human_readable: '7 days') } + let(:group) { create(:group, namespace_settings: group_settings) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } + + it_behaves_like 'expiring token', interval: 7.days + end + + context 'project expiration' do + let(:project) { create(:project, runner_token_expiration_interval: 4.days.to_i).tap(&:save!) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } + + it_behaves_like 'expiring token', interval: 4.days + end + + context 'human-readable project expiration' do + let(:project) { create(:project, runner_token_expiration_interval_human_readable: '5 days').tap(&:save!) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } + + it_behaves_like 'expiring token', interval: 5.days + end + + context 'multiple projects' do + let(:project1) { create(:project, runner_token_expiration_interval: 8.days.to_i).tap(&:save!) } + let(:project2) { create(:project, runner_token_expiration_interval: 7.days.to_i).tap(&:save!) } + let(:project3) { create(:project, runner_token_expiration_interval: 9.days.to_i).tap(&:save!) } + let(:runner) { create(:ci_runner, :project, projects: [project1, project2, project3]) } + + it_behaves_like 'expiring token', interval: 7.days + end + + context 'with project runner token expiring' do + let_it_be(:project) { create(:project, runner_token_expiration_interval: 4.days.to_i).tap(&:save!) } + + context 'project overrides system' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let(:runner) { create(:ci_runner, :project, projects: [project]) } + + it_behaves_like 'expiring token', interval: 4.days + end + + context 'system overrides project' do + before do + stub_application_setting(project_runner_token_expiration_interval: 3.days.to_i) + end + + let(:runner) { create(:ci_runner, :project, projects: [project]) } + + it_behaves_like 'expiring token', interval: 3.days + end + end + + context 'with group runner token expiring' do + let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + + context 'group overrides system' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let(:runner) { create(:ci_runner, :group, groups: [group]) } + + it_behaves_like 'expiring token', interval: 4.days + end + + context 'system overrides group' do + before do + stub_application_setting(group_runner_token_expiration_interval: 3.days.to_i) + end + + let(:runner) { create(:ci_runner, :group, groups: [group]) } + + it_behaves_like 'expiring token', interval: 3.days + end + end + + context "with group's project runner token expiring" do + let_it_be(:parent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 2.days.to_i) } + let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) } + + context 'parent group overrides subgroup' do + let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 3.days.to_i) } + let(:group) { create(:group, parent: parent_group, namespace_settings: group_settings) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } + + it_behaves_like 'expiring token', interval: 2.days + end + + context 'subgroup overrides parent group' do + let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 1.day.to_i) } + let(:group) { create(:group, parent: parent_group, namespace_settings: group_settings) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } + + it_behaves_like 'expiring token', interval: 1.day + end + end + + context "with group's project runner token expiring" do + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 2.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + + context 'group overrides project' do + let(:project) { create(:project, group: group, runner_token_expiration_interval: 3.days.to_i).tap(&:save!) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } + + it_behaves_like 'expiring token', interval: 2.days + end + + context 'project overrides group' do + let(:project) { create(:project, group: group, runner_token_expiration_interval: 1.day.to_i).tap(&:save!) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } + + it_behaves_like 'expiring token', interval: 1.day + end + end + end end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 4bdb3e0a32a..2e82a12a61a 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -289,4 +289,142 @@ RSpec.describe Ci::Build, 'TokenAuthenticatable' do expect(build.read_attribute('token')).to be_nil end end + + describe '#token_with_expiration' do + describe '#expirable?' do + subject { build.token_with_expiration.expirable? } + + it { is_expected.to eq(false) } + end + end +end + +RSpec.describe Ci::Runner, 'TokenAuthenticatable', :freeze_time do + let_it_be(:non_expirable_runner) { create(:ci_runner) } + let_it_be(:non_expired_runner) { create(:ci_runner).tap { |r| r.update!(token_expires_at: 5.seconds.from_now) } } + let_it_be(:expired_runner) { create(:ci_runner).tap { |r| r.update!(token_expires_at: 5.seconds.ago) } } + + describe '#token_expired?' do + subject { runner.token_expired? } + + context 'when enforce_runner_token_expires_at feature flag is disabled' do + before do + stub_feature_flags(enforce_runner_token_expires_at: false) + end + + context 'when runner has no token expiration' do + let(:runner) { non_expirable_runner } + + it { is_expected.to eq(false) } + end + + context 'when runner token is not expired' do + let(:runner) { non_expired_runner } + + it { is_expected.to eq(false) } + end + + context 'when runner token is expired' do + let(:runner) { expired_runner } + + it { is_expected.to eq(false) } + end + end + + context 'when enforce_runner_token_expires_at feature flag is enabled' do + before do + stub_feature_flags(enforce_runner_token_expires_at: true) + end + + context 'when runner has no token expiration' do + let(:runner) { non_expirable_runner } + + it { is_expected.to eq(false) } + end + + context 'when runner token is not expired' do + let(:runner) { non_expired_runner } + + it { is_expected.to eq(false) } + end + + context 'when runner token is expired' do + let(:runner) { expired_runner } + + it { is_expected.to eq(true) } + end + end + end + + describe '#token_with_expiration' do + describe '#token' do + subject { non_expired_runner.token_with_expiration.token } + + it { is_expected.to eq(non_expired_runner.token) } + end + + describe '#token_expires_at' do + subject { non_expired_runner.token_with_expiration.token_expires_at } + + it { is_expected.to eq(non_expired_runner.token_expires_at) } + end + + describe '#expirable?' do + subject { non_expired_runner.token_with_expiration.expirable? } + + it { is_expected.to eq(true) } + end + end + + describe '.find_by_token' do + subject { Ci::Runner.find_by_token(runner.token) } + + context 'when enforce_runner_token_expires_at feature flag is disabled' do + before do + stub_feature_flags(enforce_runner_token_expires_at: false) + end + + context 'when runner has no token expiration' do + let(:runner) { non_expirable_runner } + + it { is_expected.to eq(non_expirable_runner) } + end + + context 'when runner token is not expired' do + let(:runner) { non_expired_runner } + + it { is_expected.to eq(non_expired_runner) } + end + + context 'when runner token is expired' do + let(:runner) { expired_runner } + + it { is_expected.to eq(expired_runner) } + end + end + + context 'when enforce_runner_token_expires_at feature flag is enabled' do + before do + stub_feature_flags(enforce_runner_token_expires_at: true) + end + + context 'when runner has no token expiration' do + let(:runner) { non_expirable_runner } + + it { is_expected.to eq(non_expirable_runner) } + end + + context 'when runner token is not expired' do + let(:runner) { non_expired_runner } + + it { is_expected.to eq(non_expired_runner) } + end + + context 'when runner token is expired' do + let(:runner) { expired_runner } + + it { is_expected.to be_nil } + end + end + end end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb index b311e302a31..1772fd0ff95 100644 --- a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -23,6 +23,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do let(:options) { { encrypted: :required } } it 'finds the encrypted resource by cleartext' do + allow(model).to receive(:where) + .and_return(model) allow(model).to receive(:find_by) .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) .and_return('encrypted resource') @@ -36,6 +38,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do let(:options) { { encrypted: :optional } } it 'finds the encrypted resource by cleartext' do + allow(model).to receive(:where) + .and_return(model) allow(model).to receive(:find_by) .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) .and_return('encrypted resource') @@ -49,6 +53,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do .to receive(:find_token_authenticatable) .and_return('plaintext resource') + allow(model).to receive(:where) + .and_return(model) allow(model).to receive(:find_by) .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) .and_return(nil) @@ -62,6 +68,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do let(:options) { { encrypted: :migrating } } it 'finds the cleartext resource by cleartext' do + allow(model).to receive(:where) + .and_return(model) allow(model).to receive(:find_by) .with('some_field' => 'my-value') .and_return('cleartext resource') @@ -71,6 +79,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end it 'returns nil if resource cannot be found' do + allow(model).to receive(:where) + .and_return(model) allow(model).to receive(:find_by) .with('some_field' => 'my-value') .and_return(nil) diff --git a/spec/requests/api/ci/runner/runners_verify_post_spec.rb b/spec/requests/api/ci/runner/runners_verify_post_spec.rb index 4680076acae..038e126deaa 100644 --- a/spec/requests/api/ci/runner/runners_verify_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_verify_post_spec.rb @@ -49,6 +49,30 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do let(:expected_params) { { client_id: "runner/#{runner.id}" } } end end + + context 'when non-expired token is provided' do + subject { post api('/runners/verify'), params: { token: runner.token } } + + it 'verifies Runner credentials' do + runner["token_expires_at"] = 10.days.from_now + runner.save! + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when expired token is provided' do + subject { post api('/runners/verify'), params: { token: runner.token } } + + it 'does not verify Runner credentials' do + runner["token_expires_at"] = 10.days.ago + runner.save! + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end end end diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index 8f4e9751f12..466ae29cc4c 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -1020,7 +1020,7 @@ RSpec.describe API::Ci::Runners do end end - shared_context 'GET /groups/:id/runners' do + describe 'GET /groups/:id/runners' do context 'authorized user with maintainer privileges' do it 'returns all runners' do get api("/groups/#{group.id}/runners", user) @@ -1108,16 +1108,6 @@ RSpec.describe API::Ci::Runners do end end - it_behaves_like 'GET /groups/:id/runners' - - context 'when the FF ci_find_runners_by_ci_mirrors is disabled' do - before do - stub_feature_flags(ci_find_runners_by_ci_mirrors: false) - end - - it_behaves_like 'GET /groups/:id/runners' - end - describe 'POST /projects/:id/runners' do context 'authorized user' do let_it_be(:project_runner2) { create(:ci_runner, :project, projects: [project2]) } diff --git a/spec/rubocop/cop/migration/schedule_async_spec.rb b/spec/rubocop/cop/migration/schedule_async_spec.rb index 5f848dd9b66..09d2c77369c 100644 --- a/spec/rubocop/cop/migration/schedule_async_spec.rb +++ b/spec/rubocop/cop/migration/schedule_async_spec.rb @@ -53,6 +53,17 @@ RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do end end + context 'CiDatabaseWorker.perform_async' do + it 'adds an offense when calling `CiDatabaseWorker.peform_async`' do + expect_offense(<<~RUBY) + def up + CiDatabaseWorker.perform_async(ClazzName, "Bar", "Baz") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...] + end + RUBY + end + end + context 'BackgroundMigrationWorker.perform_in' do it 'adds an offense' do expect_offense(<<~RUBY) @@ -65,6 +76,18 @@ RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do end end + context 'CiDatabaseWorker.perform_in' do + it 'adds an offense' do + expect_offense(<<~RUBY) + def up + CiDatabaseWorker + ^^^^^^^^^^^^^^^^ Don't call [...] + .perform_in(delay, ClazzName, "Bar", "Baz") + end + RUBY + end + end + context 'BackgroundMigrationWorker.bulk_perform_async' do it 'adds an offense' do expect_offense(<<~RUBY) @@ -77,12 +100,36 @@ RSpec.describe RuboCop::Cop::Migration::ScheduleAsync do end end + context 'CiDatabaseWorker.bulk_perform_async' do + it 'adds an offense' do + expect_offense(<<~RUBY) + def up + BackgroundMigration::CiDatabaseWorker + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...] + .bulk_perform_async(jobs) + end + RUBY + end + end + context 'BackgroundMigrationWorker.bulk_perform_in' do it 'adds an offense' do expect_offense(<<~RUBY) def up - BackgroundMigrationWorker - ^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...] + ::BackgroundMigrationWorker + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...] + .bulk_perform_in(5.minutes, jobs) + end + RUBY + end + end + + context 'CiDatabaseWorker.bulk_perform_in' do + it 'adds an offense' do + expect_offense(<<~RUBY) + def up + ::BackgroundMigration::CiDatabaseWorker + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't call [...] .bulk_perform_in(5.minutes, jobs) end RUBY diff --git a/spec/rubocop/cop/scalability/bulk_perform_with_context_spec.rb b/spec/rubocop/cop/scalability/bulk_perform_with_context_spec.rb index 01afaf3acb6..74912b53d37 100644 --- a/spec/rubocop/cop/scalability/bulk_perform_with_context_spec.rb +++ b/spec/rubocop/cop/scalability/bulk_perform_with_context_spec.rb @@ -39,9 +39,15 @@ RSpec.describe RuboCop::Cop::Scalability::BulkPerformWithContext do CODE end - it "does not add an offense for scheduling BackgroundMigrations" do + it "does not add an offense for scheduling on the BackgroundMigrationWorker" do expect_no_offenses(<<~CODE) BackgroundMigrationWorker.bulk_perform_in(args) CODE end + + it "does not add an offense for scheduling on the CiDatabaseWorker" do + expect_no_offenses(<<~CODE) + BackgroundMigration::CiDatabaseWorker.bulk_perform_in(args) + CODE + end end diff --git a/spec/services/service_ping/submit_service_ping_service_spec.rb b/spec/services/service_ping/submit_service_ping_service_spec.rb index b5c28e0346a..25da02089b8 100644 --- a/spec/services/service_ping/submit_service_ping_service_spec.rb +++ b/spec/services/service_ping/submit_service_ping_service_spec.rb @@ -55,7 +55,7 @@ RSpec.describe ServicePing::SubmitService do shared_examples 'does not run' do it do expect(Gitlab::HTTP).not_to receive(:post) - expect(Gitlab::UsageData).not_to receive(:data) + expect(Gitlab::Usage::ServicePingReport).not_to receive(:for) subject.execute end @@ -118,7 +118,7 @@ RSpec.describe ServicePing::SubmitService do it 'generates service ping' do stub_response(body: with_dev_ops_score_params) - expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_call_original + expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_call_original subject.execute end @@ -151,7 +151,7 @@ RSpec.describe ServicePing::SubmitService do it 'forces a refresh of usage data statistics before submitting' do stub_response(body: with_dev_ops_score_params) - expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_call_original + expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_call_original subject.execute end @@ -167,7 +167,7 @@ RSpec.describe ServicePing::SubmitService do recorded_at = Time.current usage_data = { uuid: 'uuid', recorded_at: recorded_at } - expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_return(usage_data) + expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) subject.execute @@ -190,7 +190,7 @@ RSpec.describe ServicePing::SubmitService do recorded_at = Time.current usage_data = { uuid: 'uuid', recorded_at: recorded_at } - expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_return(usage_data) + expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) subject.execute @@ -235,7 +235,7 @@ RSpec.describe ServicePing::SubmitService do recorded_at = Time.current usage_data = { uuid: 'uuid', recorded_at: recorded_at } - expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_return(usage_data) + expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) subject.execute @@ -260,7 +260,7 @@ RSpec.describe ServicePing::SubmitService do context 'and usage data is empty string' do before do - allow(Gitlab::UsageData).to receive(:data).and_return({}) + allow(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return({}) end it_behaves_like 'does not send a blank usage ping payload' @@ -269,7 +269,7 @@ RSpec.describe ServicePing::SubmitService do context 'and usage data is nil' do before do allow(ServicePing::BuildPayloadService).to receive(:execute).and_return(nil) - allow(Gitlab::UsageData).to receive(:data).and_return(nil) + allow(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(nil) end it_behaves_like 'does not send a blank usage ping payload' @@ -282,10 +282,10 @@ RSpec.describe ServicePing::SubmitService do .and_raise(described_class::SubmissionError, 'SubmissionError') end - it 'calls UsageData .data method' do + it 'calls Gitlab::Usage::ServicePingReport .for method' do usage_data = build_usage_data - expect(Gitlab::UsageData).to receive(:data).and_return(usage_data) + expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) subject.execute end @@ -326,10 +326,10 @@ RSpec.describe ServicePing::SubmitService do end end - it 'calls UsageData .data method' do + it 'calls Gitlab::Usage::ServicePingReport .for method' do usage_data = build_usage_data - expect(Gitlab::UsageData).to receive(:data).and_return(usage_data) + expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) # SubmissionError is raised as a result of 404 in response from HTTP Request expect { subject.execute }.to raise_error(described_class::SubmissionError) |
