diff options
Diffstat (limited to 'app/services')
136 files changed, 1564 insertions, 915 deletions
diff --git a/app/services/admin/set_feature_flag_service.rb b/app/services/admin/set_feature_flag_service.rb index d72a18a6a58..3378be7eddd 100644 --- a/app/services/admin/set_feature_flag_service.rb +++ b/app/services/admin/set_feature_flag_service.rb @@ -2,82 +2,143 @@ module Admin class SetFeatureFlagService + UnknownOperationError = Class.new(StandardError) + def initialize(feature_flag_name:, params:) @name = feature_flag_name + @target = Feature::Target.new(params) @params = params + @force = params[:force] end def execute - unless params[:force] + unless force error = validate_feature_flag_name return ServiceResponse.error(message: error, reason: :invalid_feature_flag) if error end - flag_target = Feature::Target.new(params) - value = gate_value(params) - - case value - when true - enable!(flag_target) - when false - disable!(flag_target) + if target.gate_specified? + update_targets else - enable_partially!(value, params) + update_global end feature_flag = Feature.get(name) # rubocop:disable Gitlab/AvoidFeatureGet ServiceResponse.success(payload: { feature_flag: feature_flag }) - rescue Feature::Target::UnknowTargetError => e + rescue Feature::InvalidOperation => e + ServiceResponse.error(message: e.message, reason: :illegal_operation) + rescue UnknownOperationError => e + ServiceResponse.error(message: e.message, reason: :illegal_operation) + rescue Feature::Target::UnknownTargetError => e ServiceResponse.error(message: e.message, reason: :actor_not_found) end private - attr_reader :name, :params + attr_reader :name, :params, :target, :force - def enable!(flag_target) - if flag_target.gate_specified? - flag_target.targets.each { |target| Feature.enable(name, target) } - else - Feature.enable(name) + # Note: the if expressions in `update_targets` and `update_global` are order dependant. + def update_targets + target.targets.each do |target| + if enable? + enable(target) + elsif disable? + Feature.disable(name, target) + elsif opt_out? + Feature.opt_out(name, target) + elsif remove_opt_out? + remove_opt_out(target) + else + raise UnknownOperationError, "Cannot set '#{name}' to #{value.inspect} for #{target}" + end end end - def disable!(flag_target) - if flag_target.gate_specified? - flag_target.targets.each { |target| Feature.disable(name, target) } - else + def update_global + if enable? + Feature.enable(name) + elsif disable? Feature.disable(name) + elsif percentage_of_actors? + Feature.enable_percentage_of_actors(name, percentage) + elsif percentage_of_time? + Feature.enable_percentage_of_time(name, percentage) + else + msg = if key.present? + "Cannot set '#{name}' (#{key.inspect}) to #{value.inspect}" + else + "Cannot set '#{name}' to #{value.inspect}" + end + + raise UnknownOperationError, msg end end - def enable_partially!(value, params) - if params[:key] == 'percentage_of_actors' - Feature.enable_percentage_of_actors(name, value) - else - Feature.enable_percentage_of_time(name, value) + def remove_opt_out(target) + raise Feature::InvalidOperation, "No opt-out exists for #{target}" unless Feature.opted_out?(name, target) + + Feature.remove_opt_out(name, target) + end + + def enable(target) + if Feature.opted_out?(name, target) + target_name = target.respond_to?(:to_reference) ? target.to_reference : target.to_s + raise Feature::InvalidOperation, "Opt-out exists for #{target_name} - remove opt-out before enabling" end + + Feature.enable(name, target) end - def validate_feature_flag_name - # overridden in EE + def value + params[:value] end - def gate_value(params) - case params[:value] - when 'true' - true - when '0', 'false' - false - else - # https://github.com/jnunemaker/flipper/blob/master/lib/flipper/typecast.rb#L47 - if params[:value].to_s.include?('.') - params[:value].to_f - else - params[:value].to_i - end - end + def key + params[:key] + end + + def numeric_value? + params[:value].match?(/^\d+(\.\d+)?$/) + end + + def percentage + raise UnknownOperationError, "Not a percentage" unless numeric_value? + + value.to_f + end + + def percentage_of_actors? + key == 'percentage_of_actors' + end + + def percentage_of_time? + return true if key == 'percentage_of_time' + return numeric_value? if key.nil? + + false + end + + # Note: `key` is NOT considered - setting to a percentage to 0 is the same as disabling. + def disable? + value.in?(%w[0 0.0 false]) + end + + # Note: `key` is NOT considered - setting to a percentage to 100 is the same + def enable? + value.in?(%w[100 100.0 true]) + end + + def opt_out? + value == 'opt_out' + end + + def remove_opt_out? + value == 'remove_opt_out' + end + + def validate_feature_flag_name + ## Overridden in EE end end end diff --git a/app/services/bulk_imports/create_service.rb b/app/services/bulk_imports/create_service.rb index d3c6dcca588..124b5964232 100644 --- a/app/services/bulk_imports/create_service.rb +++ b/app/services/bulk_imports/create_service.rb @@ -57,11 +57,14 @@ module BulkImports bulk_import = BulkImport.create!( user: current_user, source_type: 'gitlab', - source_version: client.instance_version + source_version: client.instance_version, + source_enterprise: client.instance_enterprise ) bulk_import.create_configuration!(credentials.slice(:url, :access_token)) Array.wrap(params).each do |entity| + track_access_level(entity) + BulkImports::Entity.create!( bulk_import: bulk_import, source_type: entity[:source_type], @@ -75,6 +78,34 @@ module BulkImports end end + def track_access_level(entity) + Gitlab::Tracking.event( + self.class.name, + 'create', + label: 'import_access_level', + user: current_user, + extra: { user_role: user_role(entity[:destination_namespace]), import_type: 'bulk_import_group' } + ) + end + + def user_role(destination_namespace) + namespace = Namespace.find_by_full_path(destination_namespace) + # if there is no parent namespace we assume user will be group creator/owner + return owner_role unless destination_namespace + return owner_role unless namespace + return owner_role unless namespace.group_namespace? # user namespace + + membership = current_user.group_members.find_by(source_id: namespace.id) # rubocop:disable CodeReuse/ActiveRecord + + return 'Not a member' unless membership + + Gitlab::Access.human_access(membership.access_level) + end + + def owner_role + Gitlab::Access.human_access(Gitlab::Access::OWNER) + end + def client @client ||= BulkImports::Clients::HTTP.new( url: @credentials[:url], diff --git a/app/services/bulk_imports/file_download_service.rb b/app/services/bulk_imports/file_download_service.rb index 45f1350df92..ee499c782b4 100644 --- a/app/services/bulk_imports/file_download_service.rb +++ b/app/services/bulk_imports/file_download_service.rb @@ -31,14 +31,13 @@ module BulkImports @tmpdir = tmpdir @file_size_limit = file_size_limit @allowed_content_types = allowed_content_types + @remote_content_validated = false end def execute validate_tmpdir validate_filepath validate_url - validate_content_type - validate_content_length download_file @@ -49,7 +48,7 @@ module BulkImports private - attr_reader :configuration, :relative_url, :tmpdir, :file_size_limit, :allowed_content_types + attr_reader :configuration, :relative_url, :tmpdir, :file_size_limit, :allowed_content_types, :response_headers def download_file File.open(filepath, 'wb') do |file| @@ -58,6 +57,15 @@ module BulkImports http_client.stream(relative_url) do |chunk| next if bytes_downloaded == 0 && [301, 302, 303, 307, 308].include?(chunk.code) + @response_headers ||= Gitlab::HTTP::Response::Headers.new(chunk.http_response.to_hash) + + unless @remote_content_validated + validate_content_type + validate_content_length + + @remote_content_validated = true + end + bytes_downloaded += chunk.size validate_size!(bytes_downloaded) @@ -90,10 +98,6 @@ module BulkImports ::Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? end - def response_headers - @response_headers ||= http_client.head(relative_url).headers - end - def validate_tmpdir Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) end diff --git a/app/services/chat_names/find_user_service.rb b/app/services/chat_names/find_user_service.rb index 3dd3ba7f01c..3b204d51bab 100644 --- a/app/services/chat_names/find_user_service.rb +++ b/app/services/chat_names/find_user_service.rb @@ -2,9 +2,9 @@ module ChatNames class FindUserService - def initialize(integration, params) - @integration = integration - @params = params + def initialize(team_id, user_id) + @team_id = team_id + @user_id = user_id end def execute @@ -17,12 +17,13 @@ module ChatNames private + attr_reader :team_id, :user_id + # rubocop: disable CodeReuse/ActiveRecord def find_chat_name ChatName.find_by( - integration: @integration, - team_id: @params[:team_id], - chat_id: @params[:user_id] + team_id: team_id, + chat_id: user_id ) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/ci/append_build_trace_service.rb b/app/services/ci/append_build_trace_service.rb index 0eef0ff0e61..4432eecc24a 100644 --- a/app/services/ci/append_build_trace_service.rb +++ b/app/services/ci/append_build_trace_service.rb @@ -24,6 +24,11 @@ module Ci body_start = content_range[0].to_i body_end = body_start + body_data.bytesize + if first_debug_chunk?(body_start) + # Update the build metadata prior to appending trace content + build.enable_debug_trace! + end + if trace_size_exceeded?(body_end) build.drop(:trace_size_exceeded) @@ -45,10 +50,18 @@ module Ci delegate :project, to: :build + def first_debug_chunk?(body_start) + body_start == 0 && debug_trace + end + def stream_range params.fetch(:content_range) end + def debug_trace + params.fetch(:debug_trace, false) + end + def log_range_error(stream_size, body_end) extra = { build_id: build.id, diff --git a/app/services/ci/create_downstream_pipeline_service.rb b/app/services/ci/create_downstream_pipeline_service.rb index 25cc9045052..3d0a7fb99ea 100644 --- a/app/services/ci/create_downstream_pipeline_service.rb +++ b/app/services/ci/create_downstream_pipeline_service.rb @@ -11,24 +11,25 @@ module Ci DuplicateDownstreamPipelineError = Class.new(StandardError) MAX_NESTED_CHILDREN = 2 - MAX_HIERARCHY_SIZE = 1000 def execute(bridge) @bridge = bridge - if bridge.has_downstream_pipeline? + if @bridge.has_downstream_pipeline? Gitlab::ErrorTracking.track_exception( DuplicateDownstreamPipelineError.new, bridge_id: @bridge.id, project_id: @bridge.project_id ) - return error('Already has a downstream pipeline') + return ServiceResponse.error(message: 'Already has a downstream pipeline') end pipeline_params = @bridge.downstream_pipeline_params target_ref = pipeline_params.dig(:target_revision, :ref) - return error('Pre-conditions not met') unless ensure_preconditions!(target_ref) + return ServiceResponse.error(message: 'Pre-conditions not met') unless ensure_preconditions!(target_ref) + + return ServiceResponse.error(message: 'Can not run the bridge') unless @bridge.run service = ::Ci::CreatePipelineService.new( pipeline_params.fetch(:project), @@ -40,10 +41,7 @@ module Ci .payload log_downstream_pipeline_creation(downstream_pipeline) - - downstream_pipeline.tap do |pipeline| - update_bridge_status!(@bridge, pipeline) - end + update_bridge_status!(@bridge, downstream_pipeline) end private @@ -54,9 +52,12 @@ module Ci # If bridge uses `strategy:depend` we leave it running # and update the status when the downstream pipeline completes. subject.success! unless subject.dependent? + ServiceResponse.success(payload: pipeline) else - subject.options[:downstream_errors] = pipeline.errors.full_messages + message = pipeline.errors.full_messages + subject.options[:downstream_errors] = message subject.drop!(:downstream_pipeline_creation_failed) + ServiceResponse.error(payload: pipeline, message: message) end end rescue StateMachines::InvalidTransition => e @@ -64,6 +65,7 @@ module Ci Ci::Bridge::InvalidTransitionError.new(e.message), bridge_id: bridge.id, downstream_pipeline_id: pipeline.id) + ServiceResponse.error(payload: pipeline, message: e.message) end def ensure_preconditions!(target_ref) @@ -151,7 +153,13 @@ module Ci return false unless @bridge.triggers_downstream_pipeline? # Applies to the entire pipeline tree across all projects - @bridge.pipeline.complete_hierarchy_count >= MAX_HIERARCHY_SIZE + # A pipeline tree can be shared between multiple namespaces (customers), the limit that is used here + # is the limit of the namespace that has added a downstream pipeline to a pipeline tree. + @bridge.project.actual_limits.exceeded?(:pipeline_hierarchy_size, complete_hierarchy_count) + end + + def complete_hierarchy_count + @bridge.pipeline.complete_hierarchy_count end def config_checksum(pipeline) diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 4106dfe0ecc..9c3cc803587 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -4,8 +4,6 @@ module Ci class CreatePipelineService < BaseService attr_reader :pipeline, :logger - CreateError = Class.new(StandardError) - LOG_MAX_DURATION_THRESHOLD = 3.seconds LOG_MAX_PIPELINE_SIZE = 2_000 LOG_MAX_CREATION_THRESHOLD = 20.seconds @@ -140,25 +138,24 @@ module Ci def build_logger Gitlab::Ci::Pipeline::Logger.new(project: project) do |l| l.log_when do |observations| - observations.any? do |name, values| - values.any? && + observations.any? do |name, observation| name.to_s.end_with?('duration_s') && - values.max >= LOG_MAX_DURATION_THRESHOLD + Array(observation).max >= LOG_MAX_DURATION_THRESHOLD end end l.log_when do |observations| - values = observations['pipeline_size_count'] - next false if values.empty? + count = observations['pipeline_size_count'] + next false unless count - values.max >= LOG_MAX_PIPELINE_SIZE + count >= LOG_MAX_PIPELINE_SIZE end l.log_when do |observations| - values = observations['pipeline_creation_duration_s'] - next false if values.empty? + duration = observations['pipeline_creation_duration_s'] + next false unless duration - values.max >= LOG_MAX_CREATION_THRESHOLD + duration >= LOG_MAX_CREATION_THRESHOLD end end end diff --git a/app/services/ci/enqueue_job_service.rb b/app/services/ci/enqueue_job_service.rb new file mode 100644 index 00000000000..9e3bea3fd28 --- /dev/null +++ b/app/services/ci/enqueue_job_service.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Ci + class EnqueueJobService + attr_accessor :job, :current_user, :variables + + def initialize(job, current_user:, variables: nil) + @job = job + @current_user = current_user + @variables = variables + end + + def execute(&transition) + job.user = current_user + job.job_variables_attributes = variables if variables + + transition ||= ->(job) { job.enqueue! } + Gitlab::OptimisticLocking.retry_lock(job, name: 'ci_enqueue_job', &transition) + + ResetSkippedJobsService.new(job.project, current_user).execute(job) + + job + end + end +end diff --git a/app/services/ci/generate_kubeconfig_service.rb b/app/services/ci/generate_kubeconfig_service.rb index 347bc99dbf5..1c6aaa9d1ff 100644 --- a/app/services/ci/generate_kubeconfig_service.rb +++ b/app/services/ci/generate_kubeconfig_service.rb @@ -2,9 +2,11 @@ module Ci class GenerateKubeconfigService - def initialize(pipeline, token:) + def initialize(pipeline, token:, environment:) @pipeline = pipeline @token = token + @environment = environment + @template = Gitlab::Kubernetes::Kubeconfig::Template.new end @@ -36,10 +38,13 @@ module Ci private - attr_reader :pipeline, :token, :template + attr_reader :pipeline, :token, :environment, :template def agent_authorizations - pipeline.cluster_agent_authorizations + ::Clusters::Agents::FilterAuthorizationsService.new( + pipeline.cluster_agent_authorizations, + environment: environment + ).execute end def cluster_name diff --git a/app/services/ci/job_artifacts/create_service.rb b/app/services/ci/job_artifacts/create_service.rb index 3dc097a8603..ee9982cf3ab 100644 --- a/app/services/ci/job_artifacts/create_service.rb +++ b/app/services/ci/job_artifacts/create_service.rb @@ -16,7 +16,7 @@ module Ci def initialize(job) @job = job @project = job.project - @pipeline = job.pipeline if ::Feature.enabled?(:ci_update_unlocked_job_artifacts, @project) + @pipeline = job.pipeline end def authorize(artifact_type:, filesize: nil) @@ -85,7 +85,7 @@ module Ci expire_in: expire_in } - artifact_attributes[:locked] = pipeline.locked if ::Feature.enabled?(:ci_update_unlocked_job_artifacts, project) + artifact_attributes[:locked] = pipeline.locked artifact = Ci::JobArtifact.new( artifact_attributes.merge( diff --git a/app/services/ci/pipeline_schedule_service.rb b/app/services/ci/pipeline_schedule_service.rb index 536eaa56f9b..d320382d19f 100644 --- a/app/services/ci/pipeline_schedule_service.rb +++ b/app/services/ci/pipeline_schedule_service.rb @@ -8,7 +8,7 @@ module Ci # Ensure `next_run_at` is set properly before creating a pipeline. # Otherwise, multiple pipelines could be created in a short interval. schedule.schedule_next_run! - RunPipelineScheduleWorker.perform_async(schedule.id, schedule.owner&.id) + RunPipelineScheduleWorker.perform_async(schedule.id, current_user&.id) end end end diff --git a/app/services/ci/pipeline_schedules/calculate_next_run_service.rb b/app/services/ci/pipeline_schedules/calculate_next_run_service.rb index 6c8ccb017e9..a1b9ab5f82e 100644 --- a/app/services/ci/pipeline_schedules/calculate_next_run_service.rb +++ b/app/services/ci/pipeline_schedules/calculate_next_run_service.rb @@ -35,7 +35,7 @@ module Ci def worker_cron strong_memoize(:worker_cron) do - Gitlab::Ci::CronParser.new(worker_cron_expression, Time.zone.name) + Gitlab::Ci::CronParser.new(@schedule.worker_cron_expression, Time.zone.name) end end @@ -50,10 +50,6 @@ module Ci Gitlab::Ci::CronParser.parse_natural("every #{every_x_minutes} minutes", Time.zone.name) end end - - def worker_cron_expression - Settings.cron_jobs['pipeline_schedule_worker']['cron'] - end end end end diff --git a/app/services/ci/play_bridge_service.rb b/app/services/ci/play_bridge_service.rb index a719467253e..ffcd2b05b31 100644 --- a/app/services/ci/play_bridge_service.rb +++ b/app/services/ci/play_bridge_service.rb @@ -5,12 +5,7 @@ module Ci def execute(bridge) check_access!(bridge) - bridge.tap do |bridge| - bridge.user = current_user - bridge.enqueue! - - AfterRequeueJobService.new(project, current_user).execute(bridge) - end + Ci::EnqueueJobService.new(bridge, current_user: current_user).execute end private diff --git a/app/services/ci/play_build_service.rb b/app/services/ci/play_build_service.rb index b7aec57f3e3..8d2225aba71 100644 --- a/app/services/ci/play_build_service.rb +++ b/app/services/ci/play_build_service.rb @@ -5,17 +5,7 @@ module Ci def execute(build, job_variables_attributes = nil) check_access!(build, job_variables_attributes) - if build.can_enqueue? - build.user = current_user - build.job_variables_attributes = job_variables_attributes || [] - build.enqueue! - - AfterRequeueJobService.new(project, current_user).execute(build) - - build - else - retry_build(build) - end + Ci::EnqueueJobService.new(build, current_user: current_user, variables: job_variables_attributes || []).execute rescue StateMachines::InvalidTransition retry_build(build.reset) end diff --git a/app/services/ci/process_build_service.rb b/app/services/ci/process_build_service.rb index cb51d918fc2..a5300cfd29f 100644 --- a/app/services/ci/process_build_service.rb +++ b/app/services/ci/process_build_service.rb @@ -15,7 +15,7 @@ module Ci private def process(build) - return enqueue(build) if Feature.enabled?(:ci_retry_job_fix, project) && build.enqueue_immediately? + return enqueue(build) if build.enqueue_immediately? if build.schedulable? build.schedule diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index f11577feb88..cd879e9bc07 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -108,15 +108,13 @@ module Ci def each_build(params, &blk) queue = ::Ci::Queue::BuildQueueService.new(runner) - builds = begin - if runner.instance_type? - queue.builds_for_shared_runner - elsif runner.group_type? - queue.builds_for_group_runner - else - queue.builds_for_project_runner - end - end + builds = if runner.instance_type? + queue.builds_for_shared_runner + elsif runner.group_type? + queue.builds_for_group_runner + else + queue.builds_for_project_runner + end if runner.ref_protected? builds = queue.builds_for_protected_runner(builds) diff --git a/app/services/ci/after_requeue_job_service.rb b/app/services/ci/reset_skipped_jobs_service.rb index 4374ccd52e0..eb809b0162c 100644 --- a/app/services/ci/after_requeue_job_service.rb +++ b/app/services/ci/reset_skipped_jobs_service.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true module Ci - class AfterRequeueJobService < ::BaseService + # This service resets skipped jobs so they can be processed again. + # It affects the jobs that depend on the passed in job parameter. + class ResetSkippedJobsService < ::BaseService def execute(processable) @processable = processable diff --git a/app/services/ci/retry_job_service.rb b/app/services/ci/retry_job_service.rb index 74ebaef48b1..da0e80dfed7 100644 --- a/app/services/ci/retry_job_service.rb +++ b/app/services/ci/retry_job_service.rb @@ -28,7 +28,7 @@ module Ci check_access!(job) new_job = job.clone(current_user: current_user, new_job_variables_attributes: variables) - if Feature.enabled?(:ci_retry_job_fix, project) && enqueue_if_actionable && new_job.action? + if enqueue_if_actionable && new_job.action? new_job.set_enqueue_immediately! end @@ -64,15 +64,10 @@ module Ci next if new_job.failed? - Gitlab::OptimisticLocking.retry_lock(new_job, name: 'retry_build', &:enqueue) if Feature.disabled?( - :ci_retry_job_fix, project) + ResetSkippedJobsService.new(project, current_user).execute(job) - AfterRequeueJobService.new(project, current_user).execute(job) - - if Feature.enabled?(:ci_retry_job_fix, project) - Ci::PipelineCreation::StartPipelineService.new(job.pipeline).execute - new_job.reset - end + Ci::PipelineCreation::StartPipelineService.new(job.pipeline).execute + new_job.reset end end diff --git a/app/services/ci/test_failure_history_service.rb b/app/services/ci/test_failure_history_service.rb index 5a8072b2a0d..4f794b3d671 100644 --- a/app/services/ci/test_failure_history_service.rb +++ b/app/services/ci/test_failure_history_service.rb @@ -103,7 +103,8 @@ module Ci { unit_test_id: ci_unit_test.id, build_id: build.id, - failed_at: build.finished_at + failed_at: build.finished_at, + partition_id: build.partition_id } end end diff --git a/app/services/ci/track_failed_build_service.rb b/app/services/ci/track_failed_build_service.rb index caf7034234c..973c43a9445 100644 --- a/app/services/ci/track_failed_build_service.rb +++ b/app/services/ci/track_failed_build_service.rb @@ -6,7 +6,7 @@ # @param exit_code [Int] the resulting exit code. module Ci class TrackFailedBuildService - SCHEMA_URL = 'iglu:com.gitlab/ci_build_failed/jsonschema/1-0-0' + SCHEMA_URL = 'iglu:com.gitlab/ci_build_failed/jsonschema/1-0-1' def initialize(build:, exit_code:, failure_reason:) @build = build @@ -42,7 +42,8 @@ module Ci build_name: @build.name, build_artifact_types: @build.job_artifact_types, exit_code: @exit_code, - failure_reason: @failure_reason + failure_reason: @failure_reason, + project: @build.project_id } end end diff --git a/app/services/ci/unlock_artifacts_service.rb b/app/services/ci/unlock_artifacts_service.rb index 574cdae6480..237f1997edb 100644 --- a/app/services/ci/unlock_artifacts_service.rb +++ b/app/services/ci/unlock_artifacts_service.rb @@ -11,42 +11,21 @@ module Ci unlocked_pipeline_artifacts: 0 } - if ::Feature.enabled?(:ci_update_unlocked_job_artifacts, ci_ref.project) - loop do - unlocked_pipelines = [] - unlocked_job_artifacts = [] + loop do + unlocked_pipelines = [] + unlocked_job_artifacts = [] - ::Ci::Pipeline.transaction do - unlocked_pipelines = unlock_pipelines(ci_ref, before_pipeline) - unlocked_job_artifacts = unlock_job_artifacts(unlocked_pipelines) + ::Ci::Pipeline.transaction do + unlocked_pipelines = unlock_pipelines(ci_ref, before_pipeline) + unlocked_job_artifacts = unlock_job_artifacts(unlocked_pipelines) - results[:unlocked_pipeline_artifacts] += unlock_pipeline_artifacts(unlocked_pipelines) - end + results[:unlocked_pipeline_artifacts] += unlock_pipeline_artifacts(unlocked_pipelines) + end - break if unlocked_pipelines.empty? + break if unlocked_pipelines.empty? - results[:unlocked_pipelines] += unlocked_pipelines.length - results[:unlocked_job_artifacts] += unlocked_job_artifacts.length - end - else - query = <<~SQL.squish - UPDATE "ci_pipelines" - SET "locked" = #{::Ci::Pipeline.lockeds[:unlocked]} - WHERE "ci_pipelines"."id" in ( - #{collect_pipelines(ci_ref, before_pipeline).select(:id).to_sql} - LIMIT #{BATCH_SIZE} - FOR UPDATE SKIP LOCKED - ) - RETURNING "ci_pipelines"."id"; - SQL - - loop do - unlocked_pipelines = Ci::Pipeline.connection.exec_query(query) - - break if unlocked_pipelines.empty? - - results[:unlocked_pipelines] += unlocked_pipelines.length - end + results[:unlocked_pipelines] += unlocked_pipelines.length + results[:unlocked_job_artifacts] += unlocked_job_artifacts.length end results @@ -88,13 +67,6 @@ module Ci private - def collect_pipelines(ci_ref, before_pipeline) - pipeline_scope = ci_ref.pipelines - pipeline_scope = pipeline_scope.before_pipeline(before_pipeline) if before_pipeline - - pipeline_scope.artifacts_locked - end - def unlock_job_artifacts(pipelines) return if pipelines.empty? diff --git a/app/services/clusters/agents/filter_authorizations_service.rb b/app/services/clusters/agents/filter_authorizations_service.rb new file mode 100644 index 00000000000..68517ceec04 --- /dev/null +++ b/app/services/clusters/agents/filter_authorizations_service.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Clusters + module Agents + class FilterAuthorizationsService + def initialize(authorizations, filter_params) + @authorizations = authorizations + @filter_params = filter_params + + @environments_matcher = {} + end + + def execute + filter_by_environment(authorizations) + end + + private + + attr_reader :authorizations, :filter_params + + def filter_by_environment(auths) + return auths unless filter_by_environment? + + auths.select do |auth| + next true if auth.config['environments'].blank? + + auth.config['environments'].any? { |environment_pattern| matches_environment?(environment_pattern) } + end + end + + def filter_by_environment? + filter_params.has_key?(:environment) + end + + def environment_filter + @environment_filter ||= filter_params[:environment] + end + + def matches_environment?(environment_pattern) + return false if environment_filter.nil? + + environments_matcher(environment_pattern).match?(environment_filter) + end + + def environments_matcher(environment_pattern) + @environments_matcher[environment_pattern] ||= ::Gitlab::Ci::EnvironmentMatcher.new(environment_pattern) + end + end + end +end diff --git a/app/services/clusters/agents/refresh_authorization_service.rb b/app/services/clusters/agents/refresh_authorization_service.rb index 54b90a7304c..53b14ab54da 100644 --- a/app/services/clusters/agents/refresh_authorization_service.rb +++ b/app/services/clusters/agents/refresh_authorization_service.rb @@ -83,11 +83,7 @@ module Clusters end def allowed_projects - if group_root_ancestor? - root_ancestor.all_projects - else - ::Project.id_in(project.id) - end + root_ancestor.all_projects end def allowed_groups diff --git a/app/services/clusters/applications/base_service.rb b/app/services/clusters/applications/base_service.rb deleted file mode 100644 index c6f22cfa04c..00000000000 --- a/app/services/clusters/applications/base_service.rb +++ /dev/null @@ -1,96 +0,0 @@ -# frozen_string_literal: true - -module Clusters - module Applications - class BaseService - InvalidApplicationError = Class.new(StandardError) - - attr_reader :cluster, :current_user, :params - - def initialize(cluster, user, params = {}) - @cluster = cluster - @current_user = user - @params = params.dup - end - - def execute(request) - instantiate_application.tap do |application| - if application.has_attribute?(:hostname) - application.hostname = params[:hostname] - end - - if application.has_attribute?(:email) - application.email = params[:email] - end - - if application.has_attribute?(:stack) - application.stack = params[:stack] - end - - if application.respond_to?(:oauth_application) - application.oauth_application = create_oauth_application(application, request) - end - - if application.instance_of?(Knative) - Serverless::AssociateDomainService - .new(application, pages_domain_id: params[:pages_domain_id], creator: current_user) - .execute - end - - worker = worker_class(application) - - application.make_scheduled! - - worker.perform_async(application.name, application.id) - end - end - - protected - - def worker_class(application) - raise NotImplementedError - end - - def builder - raise NotImplementedError - end - - def project_builders - raise NotImplementedError - end - - def instantiate_application - raise_invalid_application_error if unknown_application? - - builder || raise(InvalidApplicationError, "invalid application: #{application_name}") - end - - def raise_invalid_application_error - raise(InvalidApplicationError, "invalid application: #{application_name}") - end - - def unknown_application? - Clusters::Cluster::APPLICATIONS.keys.exclude?(application_name) - end - - def application_name - params[:application] - end - - def application_class - Clusters::Cluster::APPLICATIONS[application_name] - end - - def create_oauth_application(application, request) - oauth_application_params = { - name: params[:application], - redirect_uri: application.callback_url, - scopes: application.oauth_scopes, - owner: current_user - } - - ::Applications::CreateService.new(current_user, oauth_application_params).execute(request) - end - end - end -end diff --git a/app/services/clusters/applications/check_progress_service.rb b/app/services/clusters/applications/check_progress_service.rb deleted file mode 100644 index 4a07b955f8e..00000000000 --- a/app/services/clusters/applications/check_progress_service.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -module Clusters - module Applications - class CheckProgressService < BaseHelmService - def execute - return unless operation_in_progress? - - case pod_phase - when Gitlab::Kubernetes::Pod::SUCCEEDED - on_success - when Gitlab::Kubernetes::Pod::FAILED - on_failed - else - check_timeout - end - rescue Kubeclient::HttpError => e - log_error(e) - - app.make_errored!(_('Kubernetes error: %{error_code}') % { error_code: e.error_code }) - end - - private - - def operation_in_progress? - raise NotImplementedError - end - - def on_success - raise NotImplementedError - end - - def pod_name - raise NotImplementedError - end - - def on_failed - app.make_errored!(_('Operation failed. Check pod logs for %{pod_name} for more details.') % { pod_name: pod_name }) - end - - def timed_out? - raise NotImplementedError - end - - def pod_phase - helm_api.status(pod_name) - end - end - end -end diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb deleted file mode 100644 index dffb4ce65ab..00000000000 --- a/app/services/clusters/applications/install_service.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -module Clusters - module Applications - class InstallService < BaseHelmService - def execute - return unless app.scheduled? - - app.make_installing! - - install - end - - private - - def install - log_event(:begin_install) - helm_api.install(install_command) - - log_event(:schedule_wait_for_installation) - ClusterWaitForAppInstallationWorker.perform_in( - ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) - rescue Kubeclient::HttpError => e - log_error(e) - app.make_errored!(_('Kubernetes error: %{error_code}') % { error_code: e.error_code }) - rescue StandardError => e - log_error(e) - app.make_errored!(_('Failed to install.')) - end - end - end -end diff --git a/app/services/clusters/applications/prometheus_config_service.rb b/app/services/clusters/applications/prometheus_config_service.rb deleted file mode 100644 index d39d63c874f..00000000000 --- a/app/services/clusters/applications/prometheus_config_service.rb +++ /dev/null @@ -1,155 +0,0 @@ -# frozen_string_literal: true - -module Clusters - module Applications - class PrometheusConfigService - def initialize(project, cluster, app) - @project = project - @cluster = cluster - @app = app - end - - def execute(config = {}) - if has_alerts? - generate_alert_manager(config) - else - reset_alert_manager(config) - end - end - - private - - attr_reader :project, :cluster, :app - - def reset_alert_manager(config) - config = set_alert_manager_enabled(config, false) - config.delete('alertmanagerFiles') - config['serverFiles'] ||= {} - config['serverFiles']['alerts'] = {} - - config - end - - def generate_alert_manager(config) - config = set_alert_manager_enabled(config, true) - config = set_alert_manager_files(config) - - set_alert_manager_groups(config) - end - - def set_alert_manager_enabled(config, enabled) - config['alertmanager'] ||= {} - config['alertmanager']['enabled'] = enabled - - config - end - - def set_alert_manager_files(config) - config['alertmanagerFiles'] = { - 'alertmanager.yml' => { - 'receivers' => alert_manager_receivers_params, - 'route' => alert_manager_route_params - } - } - - config - end - - def set_alert_manager_groups(config) - config['serverFiles'] ||= {} - config['serverFiles']['alerts'] ||= {} - config['serverFiles']['alerts']['groups'] ||= [] - - environments_with_alerts.each do |env_name, alerts| - index = config['serverFiles']['alerts']['groups'].find_index do |group| - group['name'] == env_name - end - - if index - config['serverFiles']['alerts']['groups'][index]['rules'] = alerts - else - config['serverFiles']['alerts']['groups'] << { - 'name' => env_name, - 'rules' => alerts - } - end - end - - config - end - - def alert_manager_receivers_params - [ - { - 'name' => 'gitlab', - 'webhook_configs' => [ - { - 'url' => notify_url, - 'send_resolved' => true, - 'http_config' => { - 'bearer_token' => alert_manager_token - } - } - ] - } - ] - end - - def alert_manager_token - app.alert_manager_token - end - - def alert_manager_route_params - { - 'receiver' => 'gitlab', - 'group_wait' => '30s', - 'group_interval' => '5m', - 'repeat_interval' => '4h' - } - end - - def notify_url - ::Gitlab::Routing.url_helpers - .notify_project_prometheus_alerts_url(project, format: :json) - end - - def has_alerts? - environments_with_alerts.values.flatten(1).any? - end - - def environments_with_alerts - @environments_with_alerts ||= - environments.each_with_object({}) do |environment, hash| - name = rule_name(environment) - hash[name] = alerts(environment) - end - end - - def rule_name(environment) - "#{environment.name}.rules" - end - - def alerts(environment) - alerts = Projects::Prometheus::AlertsFinder - .new(environment: environment) - .execute - - alerts.map do |alert| - hash = alert.to_param - hash['expr'] = substitute_query_variables(hash['expr'], environment) - hash - end - end - - def substitute_query_variables(query, environment) - result = ::Prometheus::ProxyVariableSubstitutionService.new(environment, query: query).execute - - result[:params][:query] - end - - def environments - project.environments_for_scope(cluster.environment_scope) - end - end - end -end diff --git a/app/services/clusters/applications/upgrade_service.rb b/app/services/clusters/applications/upgrade_service.rb deleted file mode 100644 index ac68e64af38..00000000000 --- a/app/services/clusters/applications/upgrade_service.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module Clusters - module Applications - class UpgradeService < BaseHelmService - def execute - return unless app.scheduled? - - app.make_updating! - - upgrade - end - - private - - def upgrade - # install_command works with upgrades too - # as it basically does `helm upgrade --install` - log_event(:begin_upgrade) - helm_api.update(install_command) - - log_event(:schedule_wait_for_upgrade) - ClusterWaitForAppInstallationWorker.perform_in( - ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) - rescue Kubeclient::HttpError => e - log_error(e) - app.make_errored!(_('Kubernetes error: %{error_code}') % { error_code: e.error_code }) - rescue StandardError => e - log_error(e) - app.make_errored!(_('Failed to upgrade.')) - end - end - end -end diff --git a/app/services/clusters/kubernetes/create_or_update_service_account_service.rb b/app/services/clusters/kubernetes/create_or_update_service_account_service.rb index eabc428d0d2..e87640f4c76 100644 --- a/app/services/clusters/kubernetes/create_or_update_service_account_service.rb +++ b/app/services/clusters/kubernetes/create_or_update_service_account_service.rb @@ -3,7 +3,7 @@ module Clusters module Kubernetes class CreateOrUpdateServiceAccountService - def initialize(kubeclient, service_account_name:, service_account_namespace:, service_account_namespace_labels: nil, token_name:, rbac:, namespace_creator: false, role_binding_name: nil) + def initialize(kubeclient, service_account_name:, service_account_namespace:, token_name:, rbac:, service_account_namespace_labels: nil, namespace_creator: false, role_binding_name: nil) @kubeclient = kubeclient @service_account_name = service_account_name @service_account_namespace = service_account_namespace diff --git a/app/services/concerns/incident_management/usage_data.rb b/app/services/concerns/incident_management/usage_data.rb index 27e60029ea3..40183085344 100644 --- a/app/services/concerns/incident_management/usage_data.rb +++ b/app/services/concerns/incident_management/usage_data.rb @@ -7,7 +7,23 @@ module IncidentManagement def track_incident_action(current_user, target, action) return unless target.incident? - track_usage_event(:"incident_management_#{action}", current_user.id) + event = "incident_management_#{action}" + track_usage_event(event, current_user.id) + + namespace = target.try(:namespace) + project = target.try(:project) + + return unless Feature.enabled?(:route_hll_to_snowplow_phase2, target.try(:namespace)) + + Gitlab::Tracking.event( + self.class.to_s, + event, + project: project, + namespace: namespace, + user: current_user, + label: 'redis_hll_counters.incident_management.incident_management_total_unique_counts_monthly', + context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: event).to_context] + ) end end end diff --git a/app/services/concerns/rate_limited_service.rb b/app/services/concerns/rate_limited_service.rb index 5d7247a5b99..fa366c1ccd0 100644 --- a/app/services/concerns/rate_limited_service.rb +++ b/app/services/concerns/rate_limited_service.rb @@ -49,8 +49,8 @@ module RateLimitedService end def evaluated_scope_for(service) - opts[:scope].each_with_object({}) do |var, all| - all[var] = service.public_send(var) # rubocop: disable GitlabSecurity/PublicSend + opts[:scope].index_with do |var| + service.public_send(var) # rubocop: disable GitlabSecurity/PublicSend end end end diff --git a/app/services/deployments/create_for_build_service.rb b/app/services/deployments/create_for_build_service.rb index 7bc0ea88910..b58aa50a66f 100644 --- a/app/services/deployments/create_for_build_service.rb +++ b/app/services/deployments/create_for_build_service.rb @@ -28,7 +28,7 @@ module Deployments def to_resource(build, environment) return build.deployment if build.deployment - return unless build.starts_environment? + return unless build.deployment_job? deployment = ::Deployment.new(attributes(build, environment)) diff --git a/app/services/design_management/generate_image_versions_service.rb b/app/services/design_management/generate_image_versions_service.rb index 3ff239b59cc..0771ada72a1 100644 --- a/app/services/design_management/generate_image_versions_service.rb +++ b/app/services/design_management/generate_image_versions_service.rb @@ -69,17 +69,15 @@ module DesignManagement # The LFS pointer file data contains an "OID" that lets us retrieve `LfsObject` # records, which have an Uploader (`LfsObjectUploader`) for the original design file. def raw_files_by_path - @raw_files_by_path ||= begin - LfsObject.for_oids(blobs_by_oid.keys).each_with_object({}) do |lfs_object, h| - blob = blobs_by_oid[lfs_object.oid] - file = lfs_object.file.file - # The `CarrierWave::SanitizedFile` is loaded without knowing the `content_type` - # of the file, due to the file not having an extension. - # - # Set the content_type from the `Blob`. - file.content_type = blob.content_type - h[blob.path] = file - end + @raw_files_by_path ||= LfsObject.for_oids(blobs_by_oid.keys).each_with_object({}) do |lfs_object, h| + blob = blobs_by_oid[lfs_object.oid] + file = lfs_object.file.file + # The `CarrierWave::SanitizedFile` is loaded without knowing the `content_type` + # of the file, due to the file not having an extension. + # + # Set the content_type from the `Blob`. + file.content_type = blob.content_type + h[blob.path] = file end end diff --git a/app/services/environments/create_for_build_service.rb b/app/services/environments/create_for_build_service.rb index c46b66ac5b3..ff4da212002 100644 --- a/app/services/environments/create_for_build_service.rb +++ b/app/services/environments/create_for_build_service.rb @@ -3,10 +3,10 @@ module Environments # This class creates an environment record for a build (a pipeline job). class CreateForBuildService - def execute(build, merge_request: nil) + def execute(build) return unless build.instance_of?(::Ci::Build) && build.has_environment_keyword? - environment = to_resource(build, merge_request) + environment = to_resource(build) if environment.persisted? build.persisted_environment = environment @@ -21,12 +21,12 @@ module Environments private # rubocop: disable Performance/ActiveRecordSubtransactionMethods - def to_resource(build, merge_request) + def to_resource(build) build.project.environments.safe_find_or_create_by(name: build.expanded_environment_name) do |environment| # Initialize the attributes at creation environment.auto_stop_in = expanded_auto_stop_in(build) environment.tier = build.environment_tier_from_options - environment.merge_request = merge_request + environment.merge_request = build.pipeline.merge_request end end # rubocop: enable Performance/ActiveRecordSubtransactionMethods diff --git a/app/services/environments/schedule_to_delete_review_apps_service.rb b/app/services/environments/schedule_to_delete_review_apps_service.rb index 041b834f11b..8e9fe3300c4 100644 --- a/app/services/environments/schedule_to_delete_review_apps_service.rb +++ b/app/services/environments/schedule_to_delete_review_apps_service.rb @@ -68,7 +68,7 @@ module Environments end def mark_for_deletion(deletable_environments) - Environment.for_id(deletable_environments).schedule_to_delete + Environment.id_in(deletable_environments).schedule_to_delete end class Result diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb index 625addaf915..2f23d47029c 100644 --- a/app/services/error_tracking/list_projects_service.rb +++ b/app/services/error_tracking/list_projects_service.rb @@ -19,19 +19,18 @@ module ErrorTracking end def project_error_tracking_setting - @project_error_tracking_setting ||= begin - (super || project.build_error_tracking_setting).tap do |setting| - setting.api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from( - api_host: params[:api_host], - organization_slug: 'org', - project_slug: 'proj' - ) - - setting.token = token(setting) - setting.enabled = true - end + (super || project.build_error_tracking_setting).tap do |setting| + setting.api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from( + api_host: params[:api_host], + organization_slug: 'org', + project_slug: 'proj' + ) + + setting.token = token(setting) + setting.enabled = true end end + strong_memoize_attr :project_error_tracking_setting def token(setting) # Use param token if not masked, otherwise use database token diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 662980fe506..bf4a26400e1 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -10,6 +10,10 @@ class EventCreateService IllegalActionError = Class.new(StandardError) + DEGIGN_EVENT_LABEL = 'usage_activity_by_stage_monthly.create.action_monthly_active_users_design_management' + MR_EVENT_LABEL = 'usage_activity_by_stage_monthly.create.merge_requests_users' + MR_EVENT_PROPERTY = 'merge_requests_users' + def open_issue(issue, current_user) create_record_event(issue, current_user, :created) end @@ -26,9 +30,11 @@ class EventCreateService create_record_event(merge_request, current_user, :created).tap do track_event(event_action: :created, event_target: MergeRequest, author_id: current_user.id) track_snowplow_event( - :created, - merge_request, - current_user + action: :created, + project: merge_request.project, + user: current_user, + label: MR_EVENT_LABEL, + property: MR_EVENT_PROPERTY ) end end @@ -37,9 +43,11 @@ class EventCreateService create_record_event(merge_request, current_user, :closed).tap do track_event(event_action: :closed, event_target: MergeRequest, author_id: current_user.id) track_snowplow_event( - :closed, - merge_request, - current_user + action: :closed, + project: merge_request.project, + user: current_user, + label: MR_EVENT_LABEL, + property: MR_EVENT_PROPERTY ) end end @@ -52,9 +60,11 @@ class EventCreateService create_record_event(merge_request, current_user, :merged).tap do track_event(event_action: :merged, event_target: MergeRequest, author_id: current_user.id) track_snowplow_event( - :merged, - merge_request, - current_user + action: :merged, + project: merge_request.project, + user: current_user, + label: MR_EVENT_LABEL, + property: MR_EVENT_PROPERTY ) end end @@ -80,11 +90,12 @@ class EventCreateService if note.is_a?(DiffNote) && note.for_merge_request? track_event(event_action: :commented, event_target: MergeRequest, author_id: current_user.id) track_snowplow_event( - :commented, - note, - current_user + action: :commented, + project: note.project, + user: current_user, + label: MR_EVENT_LABEL, + property: MR_EVENT_PROPERTY ) - end end end @@ -117,17 +128,10 @@ class EventCreateService records = create.zip([:created].cycle) + update.zip([:updated].cycle) return [] if records.empty? - if create.any? - old_track_snowplow_event(create.first, current_user, - Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION, - :create, 'design_users') - end + event_meta = { user: current_user, label: DEGIGN_EVENT_LABEL, property: Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION } + track_snowplow_event(action: :create, project: create.first.project, **event_meta) if create.any? - if update.any? - old_track_snowplow_event(update.first, current_user, - Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION, - :update, 'design_users') - end + track_snowplow_event(action: :update, project: update.first.project, **event_meta) if update.any? create_record_events(records, current_user) end @@ -135,9 +139,13 @@ class EventCreateService def destroy_designs(designs, current_user) return [] unless designs.present? - old_track_snowplow_event(designs.first, current_user, - Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION, - :destroy, 'design_users') + track_snowplow_event( + action: :destroy, + project: designs.first.project, + user: current_user, + label: DEGIGN_EVENT_LABEL, + property: Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION + ) create_record_events(designs.zip([:destroyed].cycle), current_user) end @@ -229,7 +237,8 @@ class EventCreateService namespace: namespace, user: current_user, project: project, - context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: 'action_active_users_project_repo').to_context] + property: 'project_action', + context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: 'project_action').to_context] ) end @@ -270,33 +279,18 @@ class EventCreateService Gitlab::UsageDataCounters::TrackUniqueEvents.track_event(**params) end - # This will be deleted as a part of - # https://gitlab.com/groups/gitlab-org/-/epics/8641 - # once all the events are fixed - def old_track_snowplow_event(record, current_user, category, action, label) + def track_snowplow_event(action:, project:, user:, label:, property:) return unless Feature.enabled?(:route_hll_to_snowplow_phase2) - project = record.project - Gitlab::Tracking.event( - category.to_s, - action.to_s, - label: label, - project: project, - namespace: project.namespace, - user: current_user - ) - end - - def track_snowplow_event(action, record, user) - project = record.project Gitlab::Tracking.event( self.class.to_s, action.to_s, - label: 'usage_activity_by_stage_monthly.create.merge_requests_users', + label: label, namespace: project.namespace, user: user, project: project, - context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: 'merge_requests_users').to_context] + property: property.to_s, + context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: property.to_s).to_context] ) end end diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb index 7de56c037ed..71dd9501648 100644 --- a/app/services/git/branch_hooks_service.rb +++ b/app/services/git/branch_hooks_service.rb @@ -164,16 +164,27 @@ module Git end end - def unsigned_x509_shas(commits) - CommitSignatures::X509CommitSignature.unsigned_commit_shas(commits.map(&:sha)) + def signature_types + types = [ + ::CommitSignatures::GpgSignature, + ::CommitSignatures::X509CommitSignature + ] + + types.push(::CommitSignatures::SshSignature) if Feature.enabled?(:ssh_commit_signatures, project) + + types end - def unsigned_gpg_shas(commits) - CommitSignatures::GpgSignature.unsigned_commit_shas(commits.map(&:sha)) + def unsigned_commit_shas(commits) + commit_shas = commits.map(&:sha) + + signature_types + .map { |signature| signature.unsigned_commit_shas(commit_shas) } + .reduce(&:&) end def enqueue_update_signatures - unsigned = unsigned_x509_shas(limited_commits) & unsigned_gpg_shas(limited_commits) + unsigned = unsigned_commit_shas(limited_commits) return if unsigned.empty? signable = Gitlab::Git::Commit.shas_with_signatures(project.repository, unsigned) diff --git a/app/services/groups/group_links/create_service.rb b/app/services/groups/group_links/create_service.rb index 56ddf3ec0b4..52180c39972 100644 --- a/app/services/groups/group_links/create_service.rb +++ b/app/services/groups/group_links/create_service.rb @@ -2,7 +2,7 @@ module Groups module GroupLinks - class CreateService < Groups::BaseService + class CreateService < ::Groups::BaseService include GroupLinkable def initialize(group, shared_with_group, user, params) diff --git a/app/services/groups/group_links/destroy_service.rb b/app/services/groups/group_links/destroy_service.rb index 4d74b5f32e2..d1f16775ab3 100644 --- a/app/services/groups/group_links/destroy_service.rb +++ b/app/services/groups/group_links/destroy_service.rb @@ -2,7 +2,7 @@ module Groups module GroupLinks - class DestroyService < BaseService + class DestroyService < ::Groups::BaseService def execute(one_or_more_links, skip_authorization: false) unless skip_authorization || group && can?(current_user, :admin_group_member, group) return error('Not Found', 404) diff --git a/app/services/groups/group_links/update_service.rb b/app/services/groups/group_links/update_service.rb index a1411de36d6..244ec2254a8 100644 --- a/app/services/groups/group_links/update_service.rb +++ b/app/services/groups/group_links/update_service.rb @@ -2,7 +2,7 @@ module Groups module GroupLinks - class UpdateService < BaseService + class UpdateService < ::Groups::BaseService def initialize(group_link, user = nil) super(group_link.shared_group, user) diff --git a/app/services/groups/import_export/import_service.rb b/app/services/groups/import_export/import_service.rb index 4092ded67bc..ac181245986 100644 --- a/app/services/groups/import_export/import_service.rb +++ b/app/services/groups/import_export/import_service.rb @@ -8,6 +8,7 @@ module Groups def initialize(group:, user:) @group = group @current_user = user + @user_role = user_role @shared = Gitlab::ImportExport::Shared.new(@group) @logger = Gitlab::Import::Logger.build end @@ -31,6 +32,14 @@ module Groups if valid_user_permissions? && import_file && restorers.all?(&:restore) notify_success + Gitlab::Tracking.event( + self.class.name, + 'create', + label: 'import_access_level', + user: current_user, + extra: { user_role: user_role, import_type: 'import_group_from_file' } + ) + group else notify_error! @@ -43,6 +52,15 @@ module Groups private + def user_role + # rubocop:disable CodeReuse/ActiveRecord, Style/MultilineTernaryOperator + access_level = group.parent ? + current_user&.group_members&.find_by(source_id: group.parent&.id)&.access_level : + Gitlab::Access::OWNER + Gitlab::Access.human_access(access_level) + # rubocop:enable CodeReuse/ActiveRecord, Style/MultilineTernaryOperator + end + def import_file @import_file ||= Gitlab::ImportExport::FileImporter.import( importable: group, diff --git a/app/services/import/base_service.rb b/app/services/import/base_service.rb index ab3e9c7abba..6b5adcbc39e 100644 --- a/app/services/import/base_service.rb +++ b/app/services/import/base_service.rb @@ -35,5 +35,30 @@ module Import def success(project) super().merge(project: project, status: :success) end + + def track_access_level(import_type) + Gitlab::Tracking.event( + self.class.name, + 'create', + label: 'import_access_level', + user: current_user, + extra: { user_role: user_role, import_type: import_type } + ) + end + + def user_role + if current_user.id == target_namespace.owner_id + 'Owner' + else + access_level = current_user&.group_members&.find_by(source_id: target_namespace.id)&.access_level + + case access_level + when nil + 'Not a member' + else + Gitlab::Access.human_access(access_level) + end + end + end end end diff --git a/app/services/import/bitbucket_server_service.rb b/app/services/import/bitbucket_server_service.rb index 20f6c987c92..f7f17f1e53e 100644 --- a/app/services/import/bitbucket_server_service.rb +++ b/app/services/import/bitbucket_server_service.rb @@ -19,6 +19,8 @@ module Import project = create_project(credentials) + track_access_level('bitbucket') + if project.persisted? success(project) elsif project.errors[:import_source_disabled].present? diff --git a/app/services/import/github/gists_import_service.rb b/app/services/import/github/gists_import_service.rb new file mode 100644 index 00000000000..df1bbe306e7 --- /dev/null +++ b/app/services/import/github/gists_import_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Import + module Github + class GistsImportService < ::BaseService + def initialize(user, params) + @current_user = user + @params = params + end + + def execute + return error('Import already in progress', 422) if import_status.started? + + start_import + success + end + + private + + def import_status + @import_status ||= Gitlab::GithubGistsImport::Status.new(current_user.id) + end + + def encrypted_token + Gitlab::CryptoHelper.aes256_gcm_encrypt(params[:github_access_token]) + end + + def start_import + Gitlab::GithubGistsImport::StartImportWorker.perform_async(current_user.id, encrypted_token) + import_status.start! + end + end + end +end diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index a60963e28c7..2378a4b11b1 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -13,6 +13,7 @@ module Import return context_error if context_error project = create_project(access_params, provider) + track_access_level('github') if project.persisted? store_import_settings(project) diff --git a/app/services/import/gitlab_projects/file_acquisition_strategies/file_upload.rb b/app/services/import/gitlab_projects/file_acquisition_strategies/file_upload.rb index 8bee3067d6c..1652bdab5b8 100644 --- a/app/services/import/gitlab_projects/file_acquisition_strategies/file_upload.rb +++ b/app/services/import/gitlab_projects/file_acquisition_strategies/file_upload.rb @@ -8,7 +8,7 @@ module Import validate :uploaded_file - def initialize(current_user: nil, params:) + def initialize(params:, current_user: nil) @params = params end diff --git a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb index ac58711a0ac..e179a14c497 100644 --- a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb +++ b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb @@ -21,7 +21,7 @@ module Import # whole condition of this validation: validates_with RemoteFileValidator, if: -> { validate_aws_s3? || !s3_request? } - def initialize(current_user: nil, params:) + def initialize(params:, current_user: nil) @params = params end diff --git a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file_s3.rb b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file_s3.rb index 5cbca53582d..7599343d4e1 100644 --- a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file_s3.rb +++ b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file_s3.rb @@ -25,7 +25,7 @@ module Import # we add an expiration a bit longer to ensure it won't expire during the import. URL_EXPIRATION = 28.hours.seconds - def initialize(current_user: nil, params:) + def initialize(params:, current_user: nil) @params = params end diff --git a/app/services/incident_management/incidents/create_service.rb b/app/services/incident_management/incidents/create_service.rb index f44842650b7..49019278871 100644 --- a/app/services/incident_management/incidents/create_service.rb +++ b/app/services/incident_management/incidents/create_service.rb @@ -23,7 +23,7 @@ module IncidentManagement description: description, issue_type: ISSUE_TYPE, severity: severity, - alert_management_alert: alert + alert_management_alerts: [alert].compact }, spam_params: nil ).execute diff --git a/app/services/incident_management/link_alerts/base_service.rb b/app/services/incident_management/link_alerts/base_service.rb new file mode 100644 index 00000000000..474a63ab528 --- /dev/null +++ b/app/services/incident_management/link_alerts/base_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module IncidentManagement + module LinkAlerts + class BaseService < ::BaseProjectService + private + + attr_reader :incident + + def allowed? + current_user&.can?(:admin_issue, project) + end + + def success + ServiceResponse.success(payload: { incident: incident }) + end + + def error(message) + ServiceResponse.error(message: message) + end + + def error_no_permissions + error(_('You have insufficient permissions to manage alerts for this project')) + end + end + end +end diff --git a/app/services/incident_management/link_alerts/create_service.rb b/app/services/incident_management/link_alerts/create_service.rb new file mode 100644 index 00000000000..5e5a974efdd --- /dev/null +++ b/app/services/incident_management/link_alerts/create_service.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module IncidentManagement + module LinkAlerts + class CreateService < BaseService + # @param incident [Issue] an incident to link alerts + # @param current_user [User] + # @param alert_references [[String]] a list of alert references. Can be either a short reference or URL + # Examples: + # "^alert#IID" + # "https://gitlab.com/company/project/-/alert_management/IID/details" + def initialize(incident, current_user, alert_references) + @incident = incident + @current_user = current_user + @alert_references = alert_references + + super(project: incident.project, current_user: current_user) + end + + def execute + return error_no_permissions unless allowed? + + references = extract_alerts_from_references + incident.alert_management_alerts << references if references.present? + + success + end + + private + + attr_reader :alert_references + + def extract_alerts_from_references + text = alert_references.join(' ') + extractor = Gitlab::ReferenceExtractor.new(project, current_user) + extractor.analyze(text, {}) + + extractor.alerts + end + end + end +end diff --git a/app/services/incident_management/link_alerts/destroy_service.rb b/app/services/incident_management/link_alerts/destroy_service.rb new file mode 100644 index 00000000000..baeedaf74b6 --- /dev/null +++ b/app/services/incident_management/link_alerts/destroy_service.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module IncidentManagement + module LinkAlerts + class DestroyService < BaseService + # @param incident [Issue] an incident to unlink alert from + # @param current_user [User] + # @param alert [AlertManagement::Alert] an alert to unlink from the incident + def initialize(incident, current_user, alert) + @incident = incident + @current_user = current_user + @alert = alert + + super(project: incident.project, current_user: current_user) + end + + def execute + return error_no_permissions unless allowed? + + incident.alert_management_alerts.delete(alert) + + success + end + + private + + attr_reader :alert + end + end +end diff --git a/app/services/incident_management/pager_duty/process_webhook_service.rb b/app/services/incident_management/pager_duty/process_webhook_service.rb index a49e639ea62..3ce2674616e 100644 --- a/app/services/incident_management/pager_duty/process_webhook_service.rb +++ b/app/services/incident_management/pager_duty/process_webhook_service.rb @@ -9,8 +9,8 @@ module IncidentManagement # https://developer.pagerduty.com/docs/webhooks/webhook-behavior/#size-limit PAGER_DUTY_PAYLOAD_SIZE_LIMIT = 55.kilobytes - # https://developer.pagerduty.com/docs/webhooks/v2-overview/#webhook-types - PAGER_DUTY_PROCESSABLE_EVENT_TYPES = %w(incident.trigger).freeze + # https://developer.pagerduty.com/docs/db0fa8c8984fc-overview#event-types + PAGER_DUTY_PROCESSABLE_EVENT_TYPES = %w(incident.triggered).freeze def initialize(project, payload) super(project: project) @@ -33,16 +33,18 @@ module IncidentManagement attr_reader :payload def process_incidents - pager_duty_processable_events.each do |event| - ::IncidentManagement::PagerDuty::ProcessIncidentWorker.perform_async(project.id, event['incident']) - end + event = pager_duty_processable_event + return unless event + + ::IncidentManagement::PagerDuty::ProcessIncidentWorker + .perform_async(project.id, event['incident']) end - def pager_duty_processable_events - strong_memoize(:pager_duty_processable_events) do - ::PagerDuty::WebhookPayloadParser - .call(payload.to_h) - .filter { |msg| msg['event'].to_s.in?(PAGER_DUTY_PROCESSABLE_EVENT_TYPES) } + def pager_duty_processable_event + strong_memoize(:pager_duty_processable_event) do + event = ::PagerDuty::WebhookPayloadParser.call(payload.to_h) + + event if event['event'].to_s.in?(PAGER_DUTY_PROCESSABLE_EVENT_TYPES) end end diff --git a/app/services/incident_management/timeline_events/base_service.rb b/app/services/incident_management/timeline_events/base_service.rb index 7168e2fdd38..e0ca4320091 100644 --- a/app/services/incident_management/timeline_events/base_service.rb +++ b/app/services/incident_management/timeline_events/base_service.rb @@ -5,6 +5,8 @@ module IncidentManagement class BaseService include Gitlab::Utils::UsageData + AUTOCREATE_TAGS = [TimelineEventTag::START_TIME_TAG_NAME, TimelineEventTag::END_TIME_TAG_NAME].freeze + def allowed? user&.can?(:admin_incident_management_timeline_event, incident) end @@ -24,6 +26,33 @@ module IncidentManagement def error_in_save(timeline_event) error(timeline_event.errors.full_messages.to_sentence) end + + def track_timeline_event(event, project) + namespace = project.namespace + track_usage_event(event, user.id) + + return unless Feature.enabled?(:route_hll_to_snowplow_phase2, namespace) + + Gitlab::Tracking.event( + self.class.to_s, + event, + project: project, + namespace: namespace, + user: user, + label: 'redis_hll_counters.incident_management.incident_management_total_unique_counts_monthly', + context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: event).to_context] + ) + end + + def auto_create_predefined_tags(new_tags) + new_tags = new_tags.map(&:downcase) + + tags_to_create = AUTOCREATE_TAGS.select { |tag| tag.downcase.in?(new_tags) } + + tags_to_create.each do |name| + project.incident_management_timeline_event_tags.create(name: name) + end + end end end end diff --git a/app/services/incident_management/timeline_events/create_service.rb b/app/services/incident_management/timeline_events/create_service.rb index 71ff5b64515..06e8fc32335 100644 --- a/app/services/incident_management/timeline_events/create_service.rb +++ b/app/services/incident_management/timeline_events/create_service.rb @@ -5,7 +5,6 @@ module IncidentManagement DEFAULT_ACTION = 'comment' DEFAULT_EDITABLE = false DEFAULT_AUTO_CREATED = false - AUTOCREATE_TAGS = [TimelineEventTag::START_TIME_TAG_NAME, TimelineEventTag::END_TIME_TAG_NAME].freeze class CreateService < TimelineEvents::BaseService def initialize(incident, user, params) @@ -106,7 +105,7 @@ module IncidentManagement create_timeline_event_tag_links(timeline_event, params[:timeline_event_tag_names]) - track_usage_event(:incident_management_timeline_event_created, user.id) + track_timeline_event("incident_management_timeline_event_created", project) success(timeline_event) else @@ -153,16 +152,6 @@ module IncidentManagement IncidentManagement::TimelineEventTagLink.insert_all(tag_links) if tag_links.any? end - def auto_create_predefined_tags(new_tags) - new_tags = new_tags.map(&:downcase) - - tags_to_create = AUTOCREATE_TAGS.select { |tag| tag.downcase.in?(new_tags) } - - tags_to_create.each do |name| - project.incident_management_timeline_event_tags.create(name: name) - end - end - def validate_tags(project, tag_names) return [] unless tag_names&.any? diff --git a/app/services/incident_management/timeline_events/destroy_service.rb b/app/services/incident_management/timeline_events/destroy_service.rb index e1c6bbbdb85..aba46cdda27 100644 --- a/app/services/incident_management/timeline_events/destroy_service.rb +++ b/app/services/incident_management/timeline_events/destroy_service.rb @@ -18,7 +18,7 @@ module IncidentManagement if timeline_event.destroy add_system_note(incident, user) - track_usage_event(:incident_management_timeline_event_deleted, user.id) + track_timeline_event('incident_management_timeline_event_deleted', project) success(timeline_event) else error_in_save(timeline_event) diff --git a/app/services/incident_management/timeline_events/update_service.rb b/app/services/incident_management/timeline_events/update_service.rb index 8d4e29c6857..4949a5a0bd1 100644 --- a/app/services/incident_management/timeline_events/update_service.rb +++ b/app/services/incident_management/timeline_events/update_service.rb @@ -13,21 +13,41 @@ module IncidentManagement def initialize(timeline_event, user, params) @timeline_event = timeline_event @incident = timeline_event.incident + @project = incident.project @user = user @note = params[:note] @occurred_at = params[:occurred_at] @validation_context = VALIDATION_CONTEXT + @timeline_event_tags = params[:timeline_event_tag_names] end def execute return error_no_permissions unless allowed? - timeline_event.assign_attributes(update_params) + unless timeline_event_tags.nil? + auto_create_predefined_tags(timeline_event_tags) - if timeline_event.save(context: validation_context) + # Refetches the tag objects to consider predefined tags as well + new_tags = timeline_event + .project + .incident_management_timeline_event_tags + .by_names(timeline_event_tags) + + non_existing_tags = validate_tags(new_tags) + + return error("#{_("Following tags don't exist")}: #{non_existing_tags}") if non_existing_tags.any? + end + + begin + timeline_event_saved = update_timeline_event_and_event_tags(new_tags) + rescue ActiveRecord::RecordInvalid + error_in_save(timeline_event) + end + + if timeline_event_saved add_system_note(timeline_event) - track_usage_event(:incident_management_timeline_event_edited, user.id) + track_timeline_event('incident_management_timeline_event_edited', timeline_event.project) success(timeline_event) else error_in_save(timeline_event) @@ -36,7 +56,18 @@ module IncidentManagement private - attr_reader :timeline_event, :incident, :user, :note, :occurred_at, :validation_context + attr_reader :timeline_event, :incident, :project, :user, + :note, :occurred_at, :validation_context, :timeline_event_tags + + def update_timeline_event_and_event_tags(new_tags) + ApplicationRecord.transaction do + timeline_event.timeline_event_tags = new_tags unless timeline_event_tags.nil? + + timeline_event.assign_attributes(update_params) + + timeline_event.save!(context: validation_context) + end + end def update_params { updated_by_user: user, note: note, occurred_at: occurred_at }.compact @@ -61,6 +92,10 @@ module IncidentManagement :none end + def validate_tags(new_tags) + timeline_event_tags.map(&:downcase) - new_tags.map(&:name).map(&:downcase) + end + def allowed? user&.can?(:edit_incident_management_timeline_event, timeline_event) end diff --git a/app/services/issuable/discussions_list_service.rb b/app/services/issuable/discussions_list_service.rb index 7aa0363af01..1e5e37e4e1b 100644 --- a/app/services/issuable/discussions_list_service.rb +++ b/app/services/issuable/discussions_list_service.rb @@ -16,7 +16,7 @@ module Issuable end def execute - return Note.none unless can_read_issuable? + return Note.none unless can_read_issuable_notes? notes = NotesFinder.new(current_user, params.merge({ target: issuable, project: issuable.project })) .execute.with_web_entity_associations.inc_relations_for_view.fresh @@ -39,12 +39,9 @@ module Issuable notes = prepare_notes_for_rendering(notes) - # TODO: optimize this permission check. - # Given this loads notes on a single issuable and current permission system, we should not have to check - # permission on every single note. We should be able to check permission on the given issuable or its container, - # which should result in just one permission check. Perhaps that should also either be passed to NotesFinder or - # should be done in NotesFinder, which would decide right away if it would need to return no notes - # or if it should just filter out internal notes. + # we need to check the permission on every note, because some system notes for instance can have references to + # resources that some user do not have read access, so those notes are filtered out from the list of notes. + # see Note#all_referenced_mentionables_allowed? notes = notes.select { |n| n.readable_by?(current_user) } Discussion.build_collection(notes, issuable) @@ -61,10 +58,11 @@ module Issuable end end - def can_read_issuable? + def can_read_issuable_notes? return Ability.allowed?(current_user, :read_security_resource, issuable) if issuable.is_a?(Vulnerability) - Ability.allowed?(current_user, :"read_#{issuable.to_ability_name}", issuable) + Ability.allowed?(current_user, :"read_#{issuable.to_ability_name}", issuable) && + Ability.allowed?(current_user, :read_note, issuable) end end end diff --git a/app/services/issue_links/create_service.rb b/app/services/issue_links/create_service.rb index 7f509f3b3e0..80c6af88f21 100644 --- a/app/services/issue_links/create_service.rb +++ b/app/services/issue_links/create_service.rb @@ -5,9 +5,7 @@ module IssueLinks include IncidentManagement::UsageData def linkable_issuables(issues) - @linkable_issuables ||= begin - issues.select { |issue| can?(current_user, :admin_issue_link, issue) } - end + @linkable_issuables ||= issues.select { |issue| can?(current_user, :admin_issue_link, issue) } end def previous_related_issuables diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 28ea6b0ebf8..10407e99715 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -114,6 +114,11 @@ module Issues Milestones::IssuesCountService.new(milestone).delete_cache end + + override :allowed_create_params + def allowed_create_params(params) + super(params).except(:issue_type, :work_item_type_id, :work_item_type) + end end end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index da888386e0a..4f6a859e20e 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -56,7 +56,7 @@ module Issues end def perform_incident_management_actions(issue) - resolve_alert(issue) + resolve_alerts(issue) resolve_incident(issue) end @@ -71,12 +71,17 @@ module Issues SystemNoteService.change_status(issue, issue.project, current_user, issue.state, current_commit) end - def resolve_alert(issue) - return unless alert = issue.alert_management_alert + def resolve_alerts(issue) + issue.alert_management_alerts.each { |alert| resolve_alert(alert) } + end + + def resolve_alert(alert) return if alert.resolved? + issue = alert.issue + if alert.resolve - SystemNoteService.change_alert_status(alert, current_user, " by closing incident #{issue.to_reference(project)}") + SystemNoteService.change_alert_status(alert, User.alert_bot, " because #{current_user.to_reference} closed incident #{issue.to_reference(project)}") else Gitlab::AppLogger.warn( message: 'Cannot resolve an associated Alert Management alert', diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 89b35bbab24..afad8d0c6bf 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -13,7 +13,7 @@ module Issues # spam_checking is likely to be necessary. However, if there is not a request available in scope # in the caller (for example, an issue created via email) and the required arguments to the # SpamParams constructor are not otherwise available, spam_params: must be explicitly passed as nil. - def initialize(project:, current_user: nil, params: {}, spam_params:, build_service: nil) + def initialize(project:, spam_params:, current_user: nil, params: {}, build_service: nil) @extra_params = params.delete(:extra_params) || {} super(project: project, current_user: current_user, params: params) @spam_params = spam_params diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index 6366ff4076b..f7f7d85611b 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -19,6 +19,7 @@ module Issues # to receive service desk emails on the new moved issue. update_service_desk_sent_notifications + copy_email_participants queue_copy_designs new_entity @@ -49,6 +50,18 @@ module Issues .sent_notifications.update_all(project_id: new_entity.project_id, noteable_id: new_entity.id) end + def copy_email_participants + new_attributes = { id: nil, issue_id: new_entity.id } + + new_participants = original_entity.issue_email_participants.dup + + new_participants.each do |participant| + participant.assign_attributes(new_attributes) + end + + IssueEmailParticipant.bulk_insert!(new_participants) + end + override :update_old_entity def update_old_entity super diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 0aed9e3ba40..71cc5581ae6 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -146,7 +146,7 @@ module Issues # don't enqueue immediately to prevent todos removal in case of a mistake TodosDestroyer::ConfidentialIssueWorker.perform_in(Todo::WAIT_FOR_DELETE, issue.id) if issue.confidential? create_confidentiality_note(issue) - track_usage_event(:incident_management_incident_change_confidential, current_user.id) + track_incident_action(current_user, issue, :incident_change_confidential) end end diff --git a/app/services/jira_connect/create_asymmetric_jwt_service.rb b/app/services/jira_connect/create_asymmetric_jwt_service.rb index 71aba6feddd..0f24128c20b 100644 --- a/app/services/jira_connect/create_asymmetric_jwt_service.rb +++ b/app/services/jira_connect/create_asymmetric_jwt_service.rb @@ -4,10 +4,11 @@ module JiraConnect class CreateAsymmetricJwtService ARGUMENT_ERROR_MESSAGE = 'jira_connect_installation is not a proxy installation' - def initialize(jira_connect_installation) + def initialize(jira_connect_installation, event: :installed) raise ArgumentError, ARGUMENT_ERROR_MESSAGE unless jira_connect_installation.proxy? @jira_connect_installation = jira_connect_installation + @event = event end def execute @@ -30,12 +31,18 @@ module JiraConnect def qsh_claim Atlassian::Jwt.create_query_string_hash( - @jira_connect_installation.audience_installed_event_url, + audience_event_url, 'POST', @jira_connect_installation.audience_url ) end + def audience_event_url + return @jira_connect_installation.audience_uninstalled_event_url if @event == :uninstalled + + @jira_connect_installation.audience_installed_event_url + end + def private_key @private_key ||= OpenSSL::PKey::RSA.generate(3072) end diff --git a/app/services/jira_connect_installations/proxy_lifecycle_event_service.rb b/app/services/jira_connect_installations/proxy_lifecycle_event_service.rb new file mode 100644 index 00000000000..d94d9e1324e --- /dev/null +++ b/app/services/jira_connect_installations/proxy_lifecycle_event_service.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +module JiraConnectInstallations + class ProxyLifecycleEventService + SUPPOERTED_EVENTS = %i[installed uninstalled].freeze + + def self.execute(installation, event, instance_url) + new(installation, event, instance_url).execute + end + + def initialize(installation, event, instance_url) + # To ensure the event is sent to the right instance, this makes + # a copy of the installation and assigns the instance_url + # + # The installation might be modified already with a new instance_url. + # This can be the case for an uninstalled event. + # The installation is updated first, and the uninstalled event has to be sent to + # the old instance_url. + @installation = installation.dup + @installation.instance_url = instance_url + + @event = event.to_sym + + raise(ArgumentError, "Unknown event '#{@event}'") unless SUPPOERTED_EVENTS.include?(@event) + end + + def execute + result = send_hook + + return ServiceResponse.new(status: :success) if result.code == 200 + + log_unsuccessful_response(result.code, result.body) + + ServiceResponse.error(message: { type: :response_error, code: result.code }) + rescue *Gitlab::HTTP::HTTP_ERRORS => error + ServiceResponse.error(message: { type: :network_error, message: error.message }) + end + + private + + attr_reader :installation, :event + + def send_hook + Gitlab::HTTP.post(hook_uri, body: body) + end + + def hook_uri + case event + when :installed + installation.audience_installed_event_url + when :uninstalled + installation.audience_uninstalled_event_url + end + end + + def body + return base_body unless event == :installed + + base_body.merge(installed_body) + end + + def base_body + { + clientKey: installation.client_key, + jwt: jwt_token, + eventType: event + } + end + + def installed_body + { + sharedSecret: installation.shared_secret, + baseUrl: installation.base_url + } + end + + def jwt_token + @jwt_token ||= JiraConnect::CreateAsymmetricJwtService.new(@installation, event: event).execute + end + + def log_unsuccessful_response(status_code, body) + Gitlab::IntegrationsLogger.info( + integration: 'JiraConnect', + message: 'Proxy lifecycle event received error response', + event_type: event, + status_code: status_code, + body: body + ) + end + end +end diff --git a/app/services/jira_connect_installations/update_service.rb b/app/services/jira_connect_installations/update_service.rb new file mode 100644 index 00000000000..b2b6f2a91f2 --- /dev/null +++ b/app/services/jira_connect_installations/update_service.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module JiraConnectInstallations + class UpdateService + def self.execute(installation, update_params) + new(installation, update_params).execute + end + + def initialize(installation, update_params) + @installation = installation + @update_params = update_params + end + + def execute + return update_error unless @installation.update(@update_params) + + if @installation.instance_url? + hook_result = ProxyLifecycleEventService.execute(@installation, :installed, @installation.instance_url) + + if instance_url_changed? && hook_result.error? + @installation.update!(instance_url: @installation.instance_url_before_last_save) + + return instance_installation_creation_error(hook_result.message) + end + end + + send_uninstalled_hook if instance_url_changed? + + ServiceResponse.new(status: :success) + end + + private + + def instance_url_changed? + @installation.instance_url_before_last_save != @installation.instance_url + end + + def send_uninstalled_hook + return if @installation.instance_url_before_last_save.blank? + + JiraConnect::SendUninstalledHookWorker.perform_async( + @installation.id, + @installation.instance_url_before_last_save + ) + end + + def instance_installation_creation_error(error_message) + message = if error_message[:type] == :response_error + "Could not be installed on the instance. Error response code #{error_message[:code]}" + else + 'Could not be installed on the instance. Network error' + end + + ServiceResponse.error(message: { instance_url: [message] }) + end + + def update_error + ServiceResponse.error(message: @installation.errors) + end + end +end diff --git a/app/services/jira_import/start_import_service.rb b/app/services/jira_import/start_import_service.rb index 9cd56cf339e..ef376e2f24a 100644 --- a/app/services/jira_import/start_import_service.rb +++ b/app/services/jira_import/start_import_service.rb @@ -73,7 +73,7 @@ module JiraImport jira_imports_for_project = project.jira_imports.by_jira_project_key(jira_project_key).size + 1 title = "jira-import::#{jira_project_key}-#{jira_imports_for_project}" description = "Label for issues that were imported from Jira on #{import_start_time.strftime('%Y-%m-%d %H:%M:%S')}" - color = "#{::Gitlab::Color.color_for(title)}" + color = ::Gitlab::Color.color_for(title).to_s { title: title, description: description, color: color } end diff --git a/app/services/markup/rendering_service.rb b/app/services/markup/rendering_service.rb index c4abbb6b5b0..cd89c170efa 100644 --- a/app/services/markup/rendering_service.rb +++ b/app/services/markup/rendering_service.rb @@ -2,12 +2,6 @@ module Markup class RenderingService - # Let's increase the render timeout - # For a smaller one, a test that renders the blob content statically fails - # We can consider removing this custom timeout when markup_rendering_timeout FF is removed: - # https://gitlab.com/gitlab-org/gitlab/-/issues/365358 - RENDER_TIMEOUT = 5.seconds - def initialize(text, file_name: nil, context: {}, postprocess_context: {}) @text = text @file_name = file_name @@ -19,7 +13,7 @@ module Markup return '' unless text.present? return context.delete(:rendered) if context.has_key?(:rendered) - html = file_name ? markup_unsafe : markdown_unsafe + html = markup_unsafe return '' unless html.present? @@ -29,27 +23,17 @@ module Markup private def markup_unsafe - markup = proc do - if Gitlab::MarkupHelper.gitlab_markdown?(file_name) - markdown_unsafe - elsif Gitlab::MarkupHelper.asciidoc?(file_name) - asciidoc_unsafe - elsif Gitlab::MarkupHelper.plain?(file_name) - plain_unsafe - else - other_markup_unsafe - end - end - - if Feature.enabled?(:markup_rendering_timeout, context[:project]) - Gitlab::RenderTimeout.timeout(foreground: RENDER_TIMEOUT, &markup) + return markdown_unsafe unless file_name + + if Gitlab::MarkupHelper.gitlab_markdown?(file_name) + markdown_unsafe + elsif Gitlab::MarkupHelper.asciidoc?(file_name) + asciidoc_unsafe + elsif Gitlab::MarkupHelper.plain?(file_name) + plain_unsafe else - markup.call + other_markup_unsafe end - rescue StandardError => e - Gitlab::ErrorTracking.track_exception(e, project_id: context[:project]&.id, file_name: file_name) - - ActionController::Base.helpers.simple_format(text) end def markdown_unsafe diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index f18269454e3..5afc13701e0 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -15,15 +15,15 @@ module Members @skip_auth = skip_authorization last_owner = true - in_lock("delete_members:#{member.source.class}:#{member.source.id}") do + in_lock("delete_members:#{member.source.class}:#{member.source.id}", sleep_sec: 0.1.seconds) do break if member.is_a?(GroupMember) && member.source.last_owner?(member.user) last_owner = false member.destroy - member.user&.invalidate_cache_counts end unless last_owner + member.user&.invalidate_cache_counts delete_member_associations(member, skip_subresources, unassign_issuables) end diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index 20b32dbc2a0..9e39aa94246 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -22,7 +22,7 @@ module MergeRequests def prepare_merge_request(merge_request) event_service.open_mr(merge_request, current_user) - merge_request_activity_counter.track_create_mr_action(user: current_user) + merge_request_activity_counter.track_create_mr_action(user: current_user, merge_request: merge_request) merge_request_activity_counter.track_mr_including_ci_config(user: current_user, merge_request: merge_request) notification_service.new_merge_request(merge_request, current_user) diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb index 72f398ce415..8560a15b7c4 100644 --- a/app/services/merge_requests/approval_service.rb +++ b/app/services/merge_requests/approval_service.rb @@ -13,6 +13,7 @@ module MergeRequests merge_request_activity_counter.track_approve_mr_action(user: current_user, merge_request: merge_request) trigger_merge_request_merge_status_updated(merge_request) trigger_merge_request_reviewers_updated(merge_request) + trigger_merge_request_approval_state_updated(merge_request) # Approval side effects (things not required to be done immediately but # should happen after a successful approval) should be done asynchronously diff --git a/app/services/merge_requests/assign_issues_service.rb b/app/services/merge_requests/assign_issues_service.rb index f016c16e816..c107280efb1 100644 --- a/app/services/merge_requests/assign_issues_service.rb +++ b/app/services/merge_requests/assign_issues_service.rb @@ -3,15 +3,13 @@ module MergeRequests class AssignIssuesService < BaseProjectService def assignable_issues - @assignable_issues ||= begin - if current_user == merge_request.author - closes_issues.select do |issue| - !issue.is_a?(ExternalIssue) && !issue.assignees.present? && can?(current_user, :admin_issue, issue) - end - else - [] - end - end + @assignable_issues ||= if current_user == merge_request.author + closes_issues.select do |issue| + !issue.is_a?(ExternalIssue) && !issue.assignees.present? && can?(current_user, :admin_issue, issue) + end + else + [] + end end def execute diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index e7ab2c062ee..468cadb03c7 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -59,6 +59,8 @@ module MergeRequests merge_request_activity_counter.track_users_review_requested(users: new_reviewers) merge_request_activity_counter.track_reviewers_changed_action(user: current_user) trigger_merge_request_reviewers_updated(merge_request) + + capture_suggested_reviewers_accepted(merge_request) end def cleanup_environments(merge_request) @@ -137,6 +139,7 @@ module MergeRequests end filter_reviewer(merge_request) + filter_suggested_reviewers end def filter_reviewer(merge_request) @@ -163,6 +166,10 @@ module MergeRequests end end + def filter_suggested_reviewers + # Implemented in EE + end + def merge_request_metrics_service(merge_request) MergeRequestMetricsService.new(merge_request.metrics) end @@ -253,6 +260,14 @@ module MergeRequests def trigger_merge_request_merge_status_updated(merge_request) GraphqlTriggers.merge_request_merge_status_updated(merge_request) end + + def trigger_merge_request_approval_state_updated(merge_request) + GraphqlTriggers.merge_request_approval_state_updated(merge_request) + end + + def capture_suggested_reviewers_accepted(merge_request) + # Implemented in EE + end end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index cc786ac02bd..b9a681f29db 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -224,6 +224,7 @@ module MergeRequests # def assign_title_and_description assign_description_from_repository_template + replace_variables_in_description assign_title_and_description_from_commits merge_request.title ||= title_from_issue if target_project.issues_enabled? || target_project.external_issue_tracker merge_request.title ||= source_branch.titleize.humanize @@ -318,6 +319,15 @@ module MergeRequests merge_request.description = repository_template.content end + def replace_variables_in_description + return unless merge_request.description.present? + + merge_request.description = ::Gitlab::MergeRequests::MessageGenerator.new( + merge_request: merge_request, + current_user: current_user + ).new_mr_description + end + def issue_iid strong_memoize(:issue_iid) do @params_issue_iid || begin diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 04d08f257f1..8fa80dc3513 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -39,7 +39,7 @@ module MergeRequests # open while the Gitaly RPC waits. To avoid an idle in transaction # timeout, we do this before we attempt to save the merge request. - if Feature.enabled?(:async_merge_request_diff_creation, current_user) + if Feature.enabled?(:async_merge_request_diff_creation, merge_request.target_project) merge_request.skip_ensure_merge_request_diff = true else merge_request.eager_fetch_ref! diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb index aa52349b0ee..711978dc3f7 100644 --- a/app/services/merge_requests/push_options_handler_service.rb +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -7,7 +7,7 @@ module MergeRequests attr_reader :errors, :changes, :push_options, :target_project - def initialize(project:, current_user:, params: {}, changes:, push_options:) + def initialize(project:, current_user:, changes:, push_options:, params: {}) super(project: project, current_user: current_user, params: params) @target_project = @project.default_merge_request_target diff --git a/app/services/merge_requests/remove_approval_service.rb b/app/services/merge_requests/remove_approval_service.rb index 8387c23fe3f..c0bb257eda6 100644 --- a/app/services/merge_requests/remove_approval_service.rb +++ b/app/services/merge_requests/remove_approval_service.rb @@ -19,6 +19,7 @@ module MergeRequests merge_request_activity_counter.track_unapprove_mr_action(user: current_user) trigger_merge_request_merge_status_updated(merge_request) trigger_merge_request_reviewers_updated(merge_request) + trigger_merge_request_approval_state_updated(merge_request) end success diff --git a/app/services/metrics/dashboard/grafana_metric_embed_service.rb b/app/services/metrics/dashboard/grafana_metric_embed_service.rb index e94c8d92c3a..26ccded45f8 100644 --- a/app/services/metrics/dashboard/grafana_metric_embed_service.rb +++ b/app/services/metrics/dashboard/grafana_metric_embed_service.rb @@ -51,8 +51,7 @@ module Metrics # being passed to #get_dashboard (which accepts none) ::Metrics::Dashboard::BaseService .instance_method(:get_dashboard) - .bind(self) - .call() # rubocop:disable Style/MethodCallWithoutArgsParentheses + .bind_call(self) end def cache_key(*args) diff --git a/app/services/ml/experiment_tracking/candidate_repository.rb b/app/services/ml/experiment_tracking/candidate_repository.rb index b6f87995185..1dbeb30145b 100644 --- a/app/services/ml/experiment_tracking/candidate_repository.rb +++ b/app/services/ml/experiment_tracking/candidate_repository.rb @@ -14,11 +14,15 @@ module Ml ::Ml::Candidate.with_project_id_and_iid(project.id, iid) end - def create!(experiment, start_time) - experiment.candidates.create!( + def create!(experiment, start_time, tags = nil) + candidate = experiment.candidates.create!( user: user, start_time: start_time || 0 ) + + add_tags(candidate, tags) + + candidate end def update(candidate, status, end_time) @@ -41,36 +45,21 @@ module Ml candidate.params.create!(name: name, value: value) end - def add_metrics(candidate, metric_definitions) - return unless candidate.present? - - metrics = metric_definitions.map do |metric| - { - candidate_id: candidate.id, - name: metric[:key], - value: metric[:value], - tracked_at: metric[:timestamp], - step: metric[:step], - **timestamps - } - end + def add_tag!(candidate, name, value) + candidate.metadata.create!(name: name, value: value) + end - ::Ml::CandidateMetric.insert_all(metrics, returning: false) unless metrics.empty? + def add_metrics(candidate, metric_definitions) + extra_keys = { tracked_at: :timestamp, step: :step } + insert_many(candidate, metric_definitions, ::Ml::CandidateMetric, extra_keys) end def add_params(candidate, param_definitions) - return unless candidate.present? - - parameters = param_definitions.map do |p| - { - candidate_id: candidate.id, - name: p[:key], - value: p[:value], - **timestamps - } - end + insert_many(candidate, param_definitions, ::Ml::CandidateParam) + end - ::Ml::CandidateParam.insert_all(parameters, returning: false) unless parameters.empty? + def add_tags(candidate, tag_definitions) + insert_many(candidate, tag_definitions, ::Ml::CandidateMetadata) end private @@ -80,6 +69,22 @@ module Ml { created_at: current_time, updated_at: current_time } end + + def insert_many(candidate, definitions, entity_class, extra_keys = {}) + return unless candidate.present? && definitions.present? + + entities = definitions.map do |d| + { + candidate_id: candidate.id, + name: d[:key], + value: d[:value], + **extra_keys.transform_values { |old_key| d[old_key] }, + **timestamps + } + end + + entity_class.insert_all(entities, returning: false) unless entities.empty? + end end end end diff --git a/app/services/ml/experiment_tracking/experiment_repository.rb b/app/services/ml/experiment_tracking/experiment_repository.rb index 891674adc2a..90f4cf1abec 100644 --- a/app/services/ml/experiment_tracking/experiment_repository.rb +++ b/app/services/ml/experiment_tracking/experiment_repository.rb @@ -20,10 +20,43 @@ module Ml ::Ml::Experiment.by_project_id(project.id) end - def create!(name) - ::Ml::Experiment.create!(name: name, - user: user, - project: project) + def create!(name, tags = nil) + experiment = ::Ml::Experiment.create!(name: name, + user: user, + project: project) + + add_tags(experiment, tags) + + experiment + end + + def add_tag!(experiment, key, value) + return unless experiment.present? + + experiment.metadata.create!(name: key, value: value) + end + + private + + def timestamps + current_time = Time.zone.now + + { created_at: current_time, updated_at: current_time } + end + + def add_tags(experiment, tag_definitions) + return unless experiment.present? && tag_definitions.present? + + entities = tag_definitions.map do |d| + { + experiment_id: experiment.id, + name: d[:key], + value: d[:value], + **timestamps + } + end + + ::Ml::ExperimentMetadata.insert_all(entities, returning: false) unless entities.empty? end end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 660d9891e46..550bd6d4c55 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -98,10 +98,10 @@ class NotificationService end # Notify the user when one of their personal access tokens is revoked - def access_token_revoked(user, token_name) + def access_token_revoked(user, token_name, source = nil) return unless user.can?(:receive_notifications) - mailer.access_token_revoked_email(user, token_name).deliver_later + mailer.access_token_revoked_email(user, token_name, source).deliver_later end # Notify the user when at least one of their ssh key has expired today @@ -495,13 +495,7 @@ class NotificationService def new_access_request(member) return true unless member.notifiable?(:subscription) - source = member.source - - recipients = source.access_request_approvers_to_be_notified - - if fallback_to_group_access_request_approvers?(recipients, source) - recipients = source.group.access_request_approvers_to_be_notified - end + recipients = member.source.access_request_approvers_to_be_notified return true if recipients.empty? @@ -959,12 +953,6 @@ class NotificationService mailer.member_access_requested_email(member.real_source_type, member.id, recipient.user.id).deliver_later end - def fallback_to_group_access_request_approvers?(recipients, source) - return false if recipients.present? - - source.respond_to?(:group) && source.group - end - def warn_skipping_notifications(user, object) Gitlab::AppLogger.warn(message: "Skipping sending notifications", user: user.id, klass: object.class.to_s, object_id: object.id) end diff --git a/app/services/packages/debian/process_package_file_service.rb b/app/services/packages/debian/process_package_file_service.rb new file mode 100644 index 00000000000..59e8ac3425b --- /dev/null +++ b/app/services/packages/debian/process_package_file_service.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +module Packages + module Debian + class ProcessPackageFileService + include ExclusiveLeaseGuard + include Gitlab::Utils::StrongMemoize + + SOURCE_FIELD_SPLIT_REGEX = /[ ()]/.freeze + # used by ExclusiveLeaseGuard + DEFAULT_LEASE_TIMEOUT = 1.hour.to_i.freeze + + def initialize(package_file, creator, distribution_name, component_name) + @package_file = package_file + @creator = creator + @distribution_name = distribution_name + @component_name = component_name + end + + def execute + try_obtain_lease do + validate! + + @package_file.transaction do + update_file_metadata + end + + ::Packages::Debian::GenerateDistributionWorker.perform_async(:project, package.debian_distribution.id) + end + end + + private + + def validate! + raise ArgumentError, 'package file without Debian metadata' unless @package_file.debian_file_metadatum + raise ArgumentError, 'already processed package file' unless @package_file.debian_file_metadatum.unknown? + + return if file_metadata[:file_type] == :deb || file_metadata[:file_type] == :udeb + + raise ArgumentError, "invalid package file type: #{file_metadata[:file_type]}" + end + + def update_file_metadata + ::Packages::UpdatePackageFileService.new(@package_file, package_id: package.id) + .execute + + # Force reload from database, as package has changed + @package_file.reload_package + + @package_file.debian_file_metadatum.update!( + file_type: file_metadata[:file_type], + component: @component_name, + architecture: file_metadata[:architecture], + fields: file_metadata[:fields] + ) + end + + def package + strong_memoize(:package) do + package_name = file_metadata[:fields]['Package'] + package_version = file_metadata[:fields]['Version'] + + if file_metadata[:fields]['Source'] + # "sample" or "sample (1.2.3~alpha2)" + source_field_parts = file_metadata[:fields]['Source'].split(SOURCE_FIELD_SPLIT_REGEX) + package_name = source_field_parts[0] + package_version = source_field_parts[2] || package_version + end + + params = { + 'name': package_name, + 'version': package_version, + 'distribution_name': @distribution_name + } + response = Packages::Debian::FindOrCreatePackageService.new(project, @creator, params).execute + response.payload[:package] + end + end + + def file_metadata + strong_memoize(:metadata) do + ::Packages::Debian::ExtractMetadataService.new(@package_file).execute + end + end + + def project + @package_file.package.project + end + + # used by ExclusiveLeaseGuard + def lease_key + "packages:debian:process_package_file_service:package_file:#{@package_file.id}" + end + + # used by ExclusiveLeaseGuard + def lease_timeout + DEFAULT_LEASE_TIMEOUT + end + end + end +end diff --git a/app/services/packages/rpm/parse_package_service.rb b/app/services/packages/rpm/parse_package_service.rb index 18b916a9d8b..d2751c77c5b 100644 --- a/app/services/packages/rpm/parse_package_service.rb +++ b/app/services/packages/rpm/parse_package_service.rb @@ -49,8 +49,8 @@ module Packages end def extract_static_attributes - STATIC_ATTRIBUTES.each_with_object({}) do |attribute, hash| - hash[attribute] = package_tags[attribute] + STATIC_ATTRIBUTES.index_with do |attribute| + package_tags[attribute] end end diff --git a/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb b/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb index ca5df4ce017..1733021cbb5 100644 --- a/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb +++ b/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb @@ -28,7 +28,7 @@ module PagesDomains api_order = ::Gitlab::LetsEncrypt::Client.new.load_order(acme_order.url) - # https://tools.ietf.org/html/rfc8555#section-7.1.6 - statuses diagram + # https://www.rfc-editor.org/rfc/rfc8555#section-7.1.6 - statuses diagram case api_order.status when 'ready' api_order.request_certificate(private_key: acme_order.private_key, domain: pages_domain.domain) diff --git a/app/services/pages_domains/retry_acme_order_service.rb b/app/services/pages_domains/retry_acme_order_service.rb index ef3d8ce0b67..6251c9d3615 100644 --- a/app/services/pages_domains/retry_acme_order_service.rb +++ b/app/services/pages_domains/retry_acme_order_service.rb @@ -15,7 +15,26 @@ module PagesDomains pages_domain.update!(auto_ssl_failed: false) end - PagesDomainSslRenewalWorker.perform_async(pages_domain.id) if updated + return unless updated + + PagesDomainSslRenewalWorker.perform_async(pages_domain.id) + + publish_event(pages_domain) + end + + private + + def publish_event(domain) + event = PagesDomainUpdatedEvent.new( + data: { + project_id: domain.project.id, + namespace_id: domain.project.namespace_id, + root_namespace_id: domain.project.root_namespace.id, + domain: domain.domain + } + ) + + Gitlab::EventStore.publish(event) end end end diff --git a/app/services/personal_access_tokens/revoke_service.rb b/app/services/personal_access_tokens/revoke_service.rb index 5371b6c91ef..bb5edc27340 100644 --- a/app/services/personal_access_tokens/revoke_service.rb +++ b/app/services/personal_access_tokens/revoke_service.rb @@ -4,10 +4,13 @@ module PersonalAccessTokens class RevokeService < BaseService attr_reader :token, :current_user, :group - def initialize(current_user = nil, token: nil, group: nil) + VALID_SOURCES = %w[secret_detection].freeze + + def initialize(current_user = nil, token: nil, group: nil, source: nil) @current_user = current_user @token = token @group = group + @source = source end def execute @@ -15,7 +18,7 @@ module PersonalAccessTokens if token.revoke! log_event - notification_service.access_token_revoked(token.user, token.name) + notification_service.access_token_revoked(token.user, token.name, @source) ServiceResponse.success(message: success_message) else ServiceResponse.error(message: error_message) @@ -33,11 +36,24 @@ module PersonalAccessTokens end def revocation_permitted? - Ability.allowed?(current_user, :revoke_token, token) + if current_user + Ability.allowed?(current_user, :revoke_token, token) + else + source && VALID_SOURCES.include?(source) + end + end + + def source + current_user&.username || @source end def log_event - Gitlab::AppLogger.info("PAT REVOCATION: revoked_by: '#{current_user.username}', revoked_for: '#{token.user.username}', token_id: '#{token.id}'") + Gitlab::AppLogger.info( + class: self.class.name, + message: "PAT Revoked", + revoked_by: source, + revoked_for: token.user.username, + token_id: token.id) end end end diff --git a/app/services/projects/batch_forks_count_service.rb b/app/services/projects/batch_forks_count_service.rb index 6467744a435..78663d8dad5 100644 --- a/app/services/projects/batch_forks_count_service.rb +++ b/app/services/projects/batch_forks_count_service.rb @@ -7,11 +7,9 @@ module Projects class BatchForksCountService < Projects::BatchCountService # rubocop: disable CodeReuse/ActiveRecord def global_count - @global_count ||= begin - count_service.query(project_ids) + @global_count ||= count_service.query(project_ids) .group(:forked_from_project_id) .count - end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/projects/batch_open_issues_count_service.rb b/app/services/projects/batch_open_issues_count_service.rb index d6ff2291af8..c396d7c0cfc 100644 --- a/app/services/projects/batch_open_issues_count_service.rb +++ b/app/services/projects/batch_open_issues_count_service.rb @@ -7,9 +7,7 @@ module Projects class BatchOpenIssuesCountService < Projects::BatchCountService # rubocop: disable CodeReuse/ActiveRecord def global_count - @global_count ||= begin - count_service.query(project_ids).group(:project_id).count - end + @global_count ||= count_service.query(project_ids).group(:project_id).count end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/projects/container_repository/cleanup_tags_base_service.rb b/app/services/projects/container_repository/cleanup_tags_base_service.rb index 5393c2c080d..45557d03502 100644 --- a/app/services/projects/container_repository/cleanup_tags_base_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_base_service.rb @@ -6,6 +6,8 @@ module Projects private def filter_out_latest!(tags) + return unless keep_latest + tags.reject!(&:latest?) end @@ -84,6 +86,10 @@ module Projects params['keep_n'] end + def keep_latest + params.fetch('keep_latest', true) + end + def project container_repository.project end diff --git a/app/services/projects/container_repository/destroy_service.rb b/app/services/projects/container_repository/destroy_service.rb index 83bb8624bba..6db6b449671 100644 --- a/app/services/projects/container_repository/destroy_service.rb +++ b/app/services/projects/container_repository/destroy_service.rb @@ -3,12 +3,46 @@ module Projects module ContainerRepository class DestroyService < BaseService - def execute(container_repository) + CLEANUP_TAGS_SERVICE_PARAMS = { + 'name_regex_delete' => '.*', + 'container_expiration_policy' => true, # to avoid permissions checks + 'keep_latest' => false + }.freeze + + def execute(container_repository, disable_timeout: true) return false unless can?(current_user, :update_container_image, project) # Delete tags outside of the transaction to avoid hitting an idle-in-transaction timeout - container_repository.delete_tags! - container_repository.delete_failed! unless container_repository.destroy + unless delete_tags(container_repository, disable_timeout) && + destroy_container_repository(container_repository) + container_repository.delete_failed! + end + end + + private + + def delete_tags(container_repository, disable_timeout) + service = Projects::ContainerRepository::CleanupTagsService.new( + container_repository: container_repository, + params: CLEANUP_TAGS_SERVICE_PARAMS.merge('disable_timeout' => disable_timeout) + ) + result = service.execute + return true if result[:status] == :success + + log_error(error_message(container_repository, 'error in deleting tags')) + false + end + + def destroy_container_repository(container_repository) + return true if container_repository.destroy + + log_error(error_message(container_repository, container_repository.errors.full_messages.join('. '))) + false + end + + def error_message(container_repository, message) + "Container repository with ID: #{container_repository.id} and path: #{container_repository.path}" \ + " failed with message: #{message}" end end end diff --git a/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb b/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb index e947e9575e2..b69a3cc1a2c 100644 --- a/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb @@ -18,7 +18,7 @@ module Projects container_repository.each_tags_page(page_size: TAGS_PAGE_SIZE) do |tags| execute_for_tags(tags, result) - raise TimeoutError if timeout?(start_time) + raise TimeoutError if !timeout_disabled? && timeout?(start_time) end end end @@ -72,6 +72,10 @@ module Projects def pushed_at(tag) tag.updated_at || tag.created_at end + + def timeout_disabled? + params['disable_timeout'] || false + end end end end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index c72f9b4b602..a4b473f35c6 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -317,6 +317,3 @@ module Projects end Projects::CreateService.prepend_mod_with('Projects::CreateService') - -# Measurable should be at the bottom of the ancestor chain, so it will measure execution of EE::Projects::CreateService as well -Projects::CreateService.prepend(Measurable) diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index ddbcfbb675c..a1f55f547a1 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -3,8 +3,6 @@ module Projects module ImportExport class ExportService < BaseService - prepend Measurable - def initialize(*args) super diff --git a/app/services/projects/import_export/parallel_export_service.rb b/app/services/projects/import_export/parallel_export_service.rb new file mode 100644 index 00000000000..7e4c0279b06 --- /dev/null +++ b/app/services/projects/import_export/parallel_export_service.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +module Projects + module ImportExport + class ParallelExportService + def initialize(export_job, current_user, after_export_strategy) + @export_job = export_job + @current_user = current_user + @after_export_strategy = after_export_strategy + @shared = project.import_export_shared + @logger = Gitlab::Export::Logger.build + end + + def execute + log_info('Parallel project export started') + + if save_exporters && save_export_archive + log_info('Parallel project export finished successfully') + execute_after_export_action(after_export_strategy) + else + notify_error + end + + ensure + cleanup + end + + private + + attr_reader :export_job, :current_user, :after_export_strategy, :shared, :logger + + delegate :project, to: :export_job + + def execute_after_export_action(after_export_strategy) + return if after_export_strategy.execute(current_user, project) + + notify_error + end + + def exporters + [version_saver, exported_relations_merger] + end + + def save_exporters + exporters.all? do |exporter| + log_info("Parallel project export - #{exporter.class.name} saver started") + + exporter.save + end + end + + def save_export_archive + Gitlab::ImportExport::Saver.save(exportable: project, shared: shared) + end + + def version_saver + @version_saver ||= Gitlab::ImportExport::VersionSaver.new(shared: shared) + end + + def exported_relations_merger + @relation_saver ||= Gitlab::ImportExport::Project::ExportedRelationsMerger.new( + export_job: export_job, + shared: shared) + end + + def cleanup + FileUtils.rm_rf(shared.export_path) if File.exist?(shared.export_path) + FileUtils.rm_rf(shared.archive_path) if File.exist?(shared.archive_path) + end + + def log_info(message) + logger.info( + message: message, + **log_base_data + ) + end + + def notify_error + logger.error( + message: 'Parallel project export error', + export_errors: shared.errors.join(', '), + export_job_id: export_job.id, + **log_base_data + ) + + NotificationService.new.project_not_exported(project, current_user, shared.errors) + end + + def log_base_data + { + project_id: project.id, + project_name: project.name, + project_path: project.full_path + } + end + end + end +end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 6a13b8e38c1..967a1e990b2 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -179,6 +179,3 @@ module Projects end Projects::ImportService.prepend_mod_with('Projects::ImportService') - -# Measurable should be at the bottom of the ancestor chain, so it will measure execution of EE::Projects::ImportService as well -Projects::ImportService.prepend(Measurable) diff --git a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb index c91103f897f..f7de7f98768 100644 --- a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# This service lists the download link from a remote source based on the +# This service yields operation on each download link from a remote source based on the # oids provided module Projects module LfsPointers @@ -23,29 +23,22 @@ module Projects @remote_uri = remote_uri end - # This method accepts two parameters: # - oids: hash of oids to query. The structure is { lfs_file_oid => lfs_file_size } - # - # Returns an array of LfsDownloadObject - def execute(oids) - return [] unless project&.lfs_enabled? && remote_uri && oids.present? + # Yields operation for each link batch-by-batch + def each_link(oids, &block) + return unless project&.lfs_enabled? && remote_uri && oids.present? - get_download_links_in_batches(oids) + download_links_in_batches(oids, &block) end private - def get_download_links_in_batches(oids, batch_size = REQUEST_BATCH_SIZE) - download_links = [] - + def download_links_in_batches(oids, batch_size = REQUEST_BATCH_SIZE, &block) oids.each_slice(batch_size) do |batch| - download_links += get_download_links(batch) + download_links_for(batch).each(&block) end - - download_links - rescue DownloadLinksRequestEntityTooLargeError => e - # Log this exceptions to see how open it happens + # Log this exceptions to see how often it happens Gitlab::ErrorTracking .track_exception(e, project_id: project&.id, batch_size: batch_size, oids_count: oids.count) @@ -57,7 +50,7 @@ module Projects raise DownloadLinksError, 'Unable to download due to RequestEntityTooLarge errors' end - def get_download_links(oids) + def download_links_for(oids) response = Gitlab::HTTP.post(remote_uri, body: request_body(oids), headers: headers) diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index eaf73b78c1c..26352198e5c 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -92,9 +92,15 @@ module Projects end def fetch_file(&block) + attempts ||= 1 response = Gitlab::HTTP.get(lfs_sanitized_url, download_options, &block) raise ResponseError, "Received error code #{response.code}" unless response.success? + rescue Net::OpenTimeout + raise if attempts >= 3 + + attempts += 1 + retry end def with_tmp_file diff --git a/app/services/projects/lfs_pointers/lfs_import_service.rb b/app/services/projects/lfs_pointers/lfs_import_service.rb index 3fc82f2c410..c9791041088 100644 --- a/app/services/projects/lfs_pointers/lfs_import_service.rb +++ b/app/services/projects/lfs_pointers/lfs_import_service.rb @@ -9,9 +9,7 @@ module Projects def execute return success unless project&.lfs_enabled? - lfs_objects_to_download = LfsObjectDownloadListService.new(project).execute - - lfs_objects_to_download.each do |lfs_download_object| + LfsObjectDownloadListService.new(project).each_list_item do |lfs_download_object| LfsDownloadService.new(project, lfs_download_object).execute end diff --git a/app/services/projects/lfs_pointers/lfs_object_download_list_service.rb b/app/services/projects/lfs_pointers/lfs_object_download_list_service.rb index b4872cd9442..09fec9939b9 100644 --- a/app/services/projects/lfs_pointers/lfs_object_download_list_service.rb +++ b/app/services/projects/lfs_pointers/lfs_object_download_list_service.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -# This service manages the whole worflow of discovering the Lfs files in a -# repository, linking them to the project and downloading (and linking) the non -# existent ones. +# This service discovers the Lfs files that are linked in repository, +# but not downloaded yet and yields the operation +# on each Lfs file link (url) to remote repository. module Projects module LfsPointers class LfsObjectDownloadListService < BaseService @@ -14,30 +14,31 @@ module Projects LfsObjectDownloadListError = Class.new(StandardError) - def execute - return [] unless project&.lfs_enabled? - - if external_lfs_endpoint? - # If the endpoint host is different from the import_url it means - # that the repo is using a third party service for storing the LFS files. - # In this case, we have to disable lfs in the project - disable_lfs! - - return [] - end + def each_list_item(&block) + return unless context_valid? # Downloading the required information and gathering it inside an # LfsDownloadObject for each oid - # LfsDownloadLinkListService .new(project, remote_uri: current_endpoint_uri) - .execute(missing_lfs_files) + .each_link(missing_lfs_files, &block) rescue LfsDownloadLinkListService::DownloadLinksError => e raise LfsObjectDownloadListError, "The LFS objects download list couldn't be imported. Error: #{e.message}" end private + def context_valid? + return false unless project&.lfs_enabled? + return true unless external_lfs_endpoint? + + # If the endpoint host is different from the import_url it means + # that the repo is using a third party service for storing the LFS files. + # In this case, we have to disable lfs in the project + disable_lfs! + false + end + def external_lfs_endpoint? lfsconfig_endpoint_uri && lfsconfig_endpoint_uri.host != import_uri.host end @@ -99,12 +100,10 @@ module Projects # The import url must end with '.git' here we ensure it is def default_endpoint_uri - @default_endpoint_uri ||= begin - import_uri.dup.tap do |uri| - path = uri.path.gsub(%r(/$), '') - path += '.git' unless path.ends_with?('.git') - uri.path = path + LFS_BATCH_API_ENDPOINT - end + @default_endpoint_uri ||= import_uri.dup.tap do |uri| + path = uri.path.gsub(%r(/$), '') + path += '.git' unless path.ends_with?('.git') + uri.path = path + LFS_BATCH_API_ENDPOINT end end end diff --git a/app/services/projects/refresh_build_artifacts_size_statistics_service.rb b/app/services/projects/refresh_build_artifacts_size_statistics_service.rb index 1f86e5f4ba9..8e006dc8c34 100644 --- a/app/services/projects/refresh_build_artifacts_size_statistics_service.rb +++ b/app/services/projects/refresh_build_artifacts_size_statistics_service.rb @@ -18,7 +18,7 @@ module Projects # Mark the refresh ready for another worker to pick up and process the next batch refresh.requeue!(batch.last.id) - refresh.project.statistics.delayed_increment_counter(:build_artifacts_size, total_artifacts_size) + refresh.project.statistics.increment_counter(:build_artifacts_size, total_artifacts_size) end else # Remove the refresh job from the table if there are no more diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 6a963e7fcd1..0fadd75669e 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -63,16 +63,19 @@ module Projects end def build_commit_status + stage = create_stage + GenericCommitStatus.new( user: build.user, ci_stage: stage, name: 'pages:deploy', - stage: 'deploy' + stage: 'deploy', + stage_idx: stage.position ) end # rubocop: disable Performance/ActiveRecordSubtransactionMethods - def stage + def create_stage build.pipeline.stages.safe_find_or_create_by(name: 'deploy', pipeline_id: build.pipeline.id) do |stage| stage.position = GenericCommitStatus::EXTERNAL_STAGE_IDX stage.project = build.project diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index f686f14b5b3..aca6fa91eb1 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -10,7 +10,7 @@ module Projects return success unless remote_mirror.enabled? # Blocked URLs are a hard failure, no need to attempt to retry - if Gitlab::UrlBlocker.blocked_url?(normalized_url(remote_mirror.url)) + if Gitlab::UrlBlocker.blocked_url?(normalized_url(remote_mirror.url), schemes: Project::VALID_MIRROR_PROTOCOLS) hard_retry_or_fail(remote_mirror, _('The remote mirror URL is invalid.'), tries) return error(remote_mirror.last_error) end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index f9a2c825608..301d11d841c 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -10,7 +10,6 @@ module Projects def execute build_topics remove_unallowed_params - mirror_operations_access_level_changes validate! ensure_wiki_exists if enabling_wiki? @@ -65,16 +64,36 @@ module Projects return unless changing_default_branch? previous_default_branch = project.default_branch + new_default_branch = params[:default_branch] - if project.change_head(params[:default_branch]) + if project.change_head(new_default_branch) params[:previous_default_branch] = previous_default_branch + if !project.root_ref?(new_default_branch) && has_custom_head_branch? + raise ValidationError, + format( + s_("UpdateProject|Could not set the default branch. Do you have a branch named 'HEAD' in your repository? (%{linkStart}How do I fix this?%{linkEnd})"), + linkStart: ambiguous_head_documentation_link, linkEnd: '</a>' + ).html_safe + end + after_default_branch_change(previous_default_branch) else raise ValidationError, s_("UpdateProject|Could not set the default branch") end end + def ambiguous_head_documentation_link + url = Rails.application.routes.url_helpers.help_page_path('user/project/repository/branches/index.md', anchor: 'error-ambiguous-head-branch-exists') + + format('<a href="%{url}" target="_blank" rel="noopener noreferrer">', url: url) + end + + # See issue: https://gitlab.com/gitlab-org/gitlab/-/issues/381731 + def has_custom_head_branch? + project.repository.branch_names.any? { |name| name.casecmp('head') == 0 } + end + def after_default_branch_change(previous_default_branch) # overridden by EE module end @@ -83,21 +102,6 @@ module Projects params.delete(:emails_disabled) unless can?(current_user, :set_emails_disabled, project) end - # Temporary code to sync permissions changes as operations access setting - # is being split into monitor_access_level, deployments_access_level, infrastructure_access_level. - # To be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/364240 - def mirror_operations_access_level_changes - return if Feature.enabled?(:split_operations_visibility_permissions, project) - - operations_access_level = params.dig(:project_feature_attributes, :operations_access_level) - - return if operations_access_level.nil? - - [:monitor_access_level, :infrastructure_access_level, :feature_flags_access_level, :environments_access_level].each do |key| - params[:project_feature_attributes][key] = operations_access_level - end - end - def after_update todos_features_changes = %w( issues_access_level diff --git a/app/services/protected_branches/api_service.rb b/app/services/protected_branches/api_service.rb index b8fe9bac13e..0a7777c7fed 100644 --- a/app/services/protected_branches/api_service.rb +++ b/app/services/protected_branches/api_service.rb @@ -3,11 +3,11 @@ module ProtectedBranches class ApiService < ProtectedBranches::BaseService def create - ::ProtectedBranches::CreateService.new(@project, @current_user, protected_branch_params).execute + ::ProtectedBranches::CreateService.new(project_or_group, @current_user, protected_branch_params).execute end def update(protected_branch) - ::ProtectedBranches::UpdateService.new(@project, @current_user, + ::ProtectedBranches::UpdateService.new(project_or_group, @current_user, protected_branch_params(with_defaults: false)).execute(protected_branch) end @@ -36,4 +36,4 @@ protected_branch_params(with_defaults: false)).execute(protected_branch) end end -ProtectedBranches::ApiService.prepend_mod_with('ProtectedBranches::ApiService') +ProtectedBranches::ApiService.prepend_mod diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb index d26c1b148bf..951017b2d01 100644 --- a/app/services/protected_branches/base_service.rb +++ b/app/services/protected_branches/base_service.rb @@ -2,10 +2,12 @@ module ProtectedBranches class BaseService < ::BaseService + attr_reader :project_or_group + # current_user - The user that performs the action # params - A hash of parameters - def initialize(project, current_user = nil, params = {}) - @project = project + def initialize(project_or_group, current_user = nil, params = {}) + @project_or_group = project_or_group @current_user = current_user @params = params end @@ -15,7 +17,7 @@ module ProtectedBranches end def refresh_cache - CacheService.new(@project, @current_user, @params).refresh + CacheService.new(@project_or_group, @current_user, @params).refresh end end end diff --git a/app/services/protected_branches/cache_service.rb b/app/services/protected_branches/cache_service.rb index 66ca549c508..af8c9ce74bb 100644 --- a/app/services/protected_branches/cache_service.rb +++ b/app/services/protected_branches/cache_service.rb @@ -66,13 +66,18 @@ module ProtectedBranches log_error( 'class' => self.class.name, 'message' => "Cache mismatch '#{encoded_ref_name}': cached value: #{cached_value}, real value: #{real_value}", - 'project_id' => @project.id, - 'project_path' => @project.full_path + 'record_class' => project_or_group.class.name, + 'record_id' => project_or_group.id, + 'record_path' => project_or_group.full_path ) end def redis_key - @redis_key ||= [CACHE_ROOT_KEY, @project.id].join(':') + @redis_key ||= if Feature.enabled?(:group_protected_branches) + [CACHE_ROOT_KEY, project_or_group.class.name, project_or_group.id].join(':') + else + [CACHE_ROOT_KEY, project_or_group.id].join(':') + end end def metrics diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb index 903addf7afc..46585e0b65d 100644 --- a/app/services/protected_branches/create_service.rb +++ b/app/services/protected_branches/create_service.rb @@ -23,9 +23,9 @@ module ProtectedBranches end def protected_branch - @protected_branch ||= project.protected_branches.new(params) + @protected_branch ||= project_or_group.protected_branches.new(params) end end end -ProtectedBranches::CreateService.prepend_mod_with('ProtectedBranches::CreateService') +ProtectedBranches::CreateService.prepend_mod diff --git a/app/services/protected_branches/destroy_service.rb b/app/services/protected_branches/destroy_service.rb index 01d3b68314f..a32a867491e 100644 --- a/app/services/protected_branches/destroy_service.rb +++ b/app/services/protected_branches/destroy_service.rb @@ -10,4 +10,4 @@ module ProtectedBranches end end -ProtectedBranches::DestroyService.prepend_mod_with('ProtectedBranches::DestroyService') +ProtectedBranches::DestroyService.prepend_mod diff --git a/app/services/protected_branches/legacy_api_create_service.rb b/app/services/protected_branches/legacy_api_create_service.rb index aef99a860a0..f662d9d1bf0 100644 --- a/app/services/protected_branches/legacy_api_create_service.rb +++ b/app/services/protected_branches/legacy_api_create_service.rb @@ -24,7 +24,7 @@ module ProtectedBranches @params.merge!(push_access_levels_attributes: [{ access_level: push_access_level }], merge_access_levels_attributes: [{ access_level: merge_access_level }]) - service = ProtectedBranches::CreateService.new(@project, @current_user, @params) + service = ProtectedBranches::CreateService.new(project_or_group, @current_user, @params) service.execute end end diff --git a/app/services/protected_branches/legacy_api_update_service.rb b/app/services/protected_branches/legacy_api_update_service.rb index 8ff6c4bd734..b144797ab6d 100644 --- a/app/services/protected_branches/legacy_api_update_service.rb +++ b/app/services/protected_branches/legacy_api_update_service.rb @@ -30,7 +30,7 @@ module ProtectedBranches params[:merge_access_levels_attributes] = [{ access_level: Gitlab::Access::MAINTAINER }] end - service = ProtectedBranches::UpdateService.new(project, current_user, params) + service = ProtectedBranches::UpdateService.new(project_or_group, current_user, params) service.execute(protected_branch) end end diff --git a/app/services/protected_branches/update_service.rb b/app/services/protected_branches/update_service.rb index c155e0022f5..4b54bf92989 100644 --- a/app/services/protected_branches/update_service.rb +++ b/app/services/protected_branches/update_service.rb @@ -19,4 +19,4 @@ module ProtectedBranches end end -ProtectedBranches::UpdateService.prepend_mod_with('ProtectedBranches::UpdateService') +ProtectedBranches::UpdateService.prepend_mod diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 1d7c5d2c80a..f1e4dac8835 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -158,15 +158,15 @@ module QuickActions end def map_commands(commands, method) - commands.map do |name, arg| - definition = self.class.definition_by_name(name) + commands.map do |name_or_alias, arg| + definition = self.class.definition_by_name(name_or_alias) next unless definition case method when :explain definition.explain(self, arg) when :execute_message - @execution_message[name.to_sym] || definition.execute_message(self, arg) + @execution_message[definition.name.to_sym] || definition.execute_message(self, arg) end end.compact end diff --git a/app/services/repositories/housekeeping_service.rb b/app/services/repositories/housekeeping_service.rb index de80390e60b..e12d69807f2 100644 --- a/app/services/repositories/housekeeping_service.rb +++ b/app/services/repositories/housekeeping_service.rb @@ -84,7 +84,11 @@ module Repositories end def period_match? - [gc_period, full_repack_period, repack_period, PACK_REFS_PERIOD].any? { |period| pushes_since_gc % period == 0 } + if Feature.enabled?(:optimized_housekeeping) + pushes_since_gc % repack_period == 0 + else + [gc_period, full_repack_period, repack_period, PACK_REFS_PERIOD].any? { |period| pushes_since_gc % period == 0 } + end end def housekeeping_enabled? diff --git a/app/services/search_service.rb b/app/services/search_service.rb index f38522b9764..403a2f077b0 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -45,9 +45,9 @@ class SearchService end def show_snippets? - return @show_snippets if defined?(@show_snippets) - - @show_snippets = params[:snippets] == 'true' + strong_memoize(:show_snippets) do + params[:snippets] == 'true' + end end delegate :scope, to: :search_service diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb index 5cadff42958..a62d5290271 100644 --- a/app/services/snippets/create_service.rb +++ b/app/services/snippets/create_service.rb @@ -4,7 +4,7 @@ module Snippets class CreateService < Snippets::BaseService # NOTE: For Issues::CreateService, we require the spam_params and do not default it to nil, because # spam_checking is likely to be necessary. - def initialize(project:, current_user: nil, params: {}, spam_params:) + def initialize(project:, spam_params:, current_user: nil, params: {}) super(project: project, current_user: current_user, params: params) @spam_params = spam_params end diff --git a/app/services/system_notes/commit_service.rb b/app/services/system_notes/commit_service.rb index c89998f77c7..592351079aa 100644 --- a/app/services/system_notes/commit_service.rb +++ b/app/services/system_notes/commit_service.rb @@ -81,12 +81,10 @@ module SystemNotes commit_ids = if count == 1 existing_commits.first.short_id + elsif oldrev && !Gitlab::Git.blank_ref?(oldrev) + "#{Commit.truncate_sha(oldrev)}...#{existing_commits.last.short_id}" else - if oldrev && !Gitlab::Git.blank_ref?(oldrev) - "#{Commit.truncate_sha(oldrev)}...#{existing_commits.last.short_id}" - else - "#{existing_commits.first.short_id}..#{existing_commits.last.short_id}" - end + "#{existing_commits.first.short_id}..#{existing_commits.last.short_id}" end commits_text = "#{count} commit".pluralize(count) diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb index 082fa1447fc..8e20ffc2a52 100644 --- a/app/services/task_list_toggle_service.rb +++ b/app/services/task_list_toggle_service.rb @@ -44,8 +44,8 @@ class TaskListToggleService # any `[ ]` or `[x]` in the middle of the text if currently_checked markdown_task.sub!(Taskable::COMPLETE_PATTERN, '[ ]') unless toggle_as_checked - else - markdown_task.sub!(Taskable::INCOMPLETE_PATTERN, '[x]') if toggle_as_checked + elsif toggle_as_checked + markdown_task.sub!(Taskable::INCOMPLETE_PATTERN, '[x]') end source_lines[source_line_index] = markdown_task diff --git a/app/services/timelogs/base_service.rb b/app/services/timelogs/base_service.rb index e09264864fd..712a0a4f128 100644 --- a/app/services/timelogs/base_service.rb +++ b/app/services/timelogs/base_service.rb @@ -22,9 +22,9 @@ module Timelogs end def error_in_save(timelog) - return error(_("Failed to save timelog")) if timelog.errors.empty? + return error(_("Failed to save timelog"), 404) if timelog.errors.empty? - error(timelog.errors.full_messages.to_sentence) + error(timelog.errors.full_messages.to_sentence, 404) end end end diff --git a/app/services/timelogs/create_service.rb b/app/services/timelogs/create_service.rb index 12181cec20a..19428864fa9 100644 --- a/app/services/timelogs/create_service.rb +++ b/app/services/timelogs/create_service.rb @@ -21,6 +21,9 @@ module Timelogs }, 404) end + return error(_("Spent at can't be a future date and time."), 404) if spent_at.future? + return error(_("Time spent can't be zero."), 404) if time_spent == 0 + issue = issuable if issuable.is_a?(Issue) merge_request = issuable if issuable.is_a?(MergeRequest) diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 06352d36215..9ae31f8ac58 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -166,8 +166,9 @@ class TodoService # When user marks a target as todo def mark_todo(target, current_user) - attributes = attributes_for_todo(target.project, target, current_user, Todo::MARKED) - create_todos(current_user, attributes) + project = target.project + attributes = attributes_for_todo(project, target, current_user, Todo::MARKED) + create_todos(current_user, attributes, project&.namespace, project) end def todo_exist?(issuable, current_user) @@ -214,13 +215,32 @@ class TodoService end def create_request_review_todo(target, author, reviewers) - attributes = attributes_for_todo(target.project, target, author, Todo::REVIEW_REQUESTED) - create_todos(reviewers, attributes) + project = target.project + attributes = attributes_for_todo(project, target, author, Todo::REVIEW_REQUESTED) + create_todos(reviewers, attributes, project.namespace, project) + end + + def create_member_access_request(member) + source = member.source + attributes = attributes_for_access_request_todos(source, member.user, Todo::MEMBER_ACCESS_REQUESTED) + + approvers = source.access_request_approvers_to_be_notified.map(&:user) + return true if approvers.empty? + + if source.instance_of? Project + project = source + namespace = project.namespace + else + project = nil + namespace = source + end + + create_todos(approvers, attributes, namespace, project) end private - def create_todos(users, attributes) + def create_todos(users, attributes, namespace, project) users = Array(users) return if users.empty? @@ -246,7 +266,7 @@ class TodoService todos = users.map do |user| issue_type = attributes.delete(:issue_type) - track_todo_creation(user, issue_type) + track_todo_creation(user, issue_type, namespace, project) Todo.create(attributes.merge(user_id: user.id)) end @@ -286,9 +306,10 @@ class TodoService def create_assignment_todo(target, author, old_assignees = []) if target.assignees.any? + project = target.project assignees = target.assignees - old_assignees - attributes = attributes_for_todo(target.project, target, author, Todo::ASSIGNED) - create_todos(assignees, attributes) + attributes = attributes_for_todo(project, target, author, Todo::ASSIGNED) + create_todos(assignees, attributes, project.namespace, project) end end @@ -303,22 +324,24 @@ class TodoService # Create Todos for directly addressed users directly_addressed_users = filter_directly_addressed_users(parent, note || target, author, skip_users) attributes = attributes_for_todo(parent, target, author, Todo::DIRECTLY_ADDRESSED, note) - create_todos(directly_addressed_users, attributes) + create_todos(directly_addressed_users, attributes, parent&.namespace, parent) # Create Todos for mentioned users mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users + directly_addressed_users) attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note) - create_todos(mentioned_users, attributes) + create_todos(mentioned_users, attributes, parent&.namespace, parent) end def create_build_failed_todo(merge_request, todo_author) - attributes = attributes_for_todo(merge_request.project, merge_request, todo_author, Todo::BUILD_FAILED) - create_todos(todo_author, attributes) + project = merge_request.project + attributes = attributes_for_todo(project, merge_request, todo_author, Todo::BUILD_FAILED) + create_todos(todo_author, attributes, project.namespace, project) end def create_unmergeable_todo(merge_request, todo_author) - attributes = attributes_for_todo(merge_request.project, merge_request, todo_author, Todo::UNMERGEABLE) - create_todos(todo_author, attributes) + project = merge_request.project + attributes = attributes_for_todo(project, merge_request, todo_author, Todo::UNMERGEABLE) + create_todos(todo_author, attributes, project.namespace, project) end def attributes_for_target(target) @@ -382,10 +405,37 @@ class TodoService PendingTodosFinder.new(users, criteria).execute end - def track_todo_creation(user, issue_type) + def track_todo_creation(user, issue_type, namespace, project) return unless issue_type == 'incident' - track_usage_event(:incident_management_incident_todo, user.id) + event = "incident_management_incident_todo" + track_usage_event(event, user.id) + + return unless Feature.enabled?(:route_hll_to_snowplow_phase2, namespace) + + Gitlab::Tracking.event( + self.class.to_s, + event, + project: project, + namespace: namespace, + user: user, + label: 'redis_hll_counters.incident_management.incident_management_total_unique_counts_monthly', + context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: event).to_context] + ) + end + + def attributes_for_access_request_todos(source, author, action, note = nil) + attributes = { + target_id: source.id, + target_type: source.class.polymorphic_name, + author_id: author.id, + action: action, + note: note + } + + attributes[:group_id] = source.id unless source.instance_of? Project + + attributes end end diff --git a/app/services/users/approve_service.rb b/app/services/users/approve_service.rb index 15486ddcd43..353456c545d 100644 --- a/app/services/users/approve_service.rb +++ b/app/services/users/approve_service.rb @@ -42,7 +42,7 @@ module Users end def log_event(user) - Gitlab::AppLogger.info(message: "User instance access request approved", user: "#{user.username}", email: "#{user.email}", approved_by: "#{current_user.username}", ip_address: "#{current_user.current_sign_in_ip}") + Gitlab::AppLogger.info(message: "User instance access request approved", user: user.username.to_s, email: user.email.to_s, approved_by: current_user.username.to_s, ip_address: current_user.current_sign_in_ip.to_s) end end end diff --git a/app/services/users/assigned_issues_count_service.rb b/app/services/users/assigned_issues_count_service.rb new file mode 100644 index 00000000000..6590902587d --- /dev/null +++ b/app/services/users/assigned_issues_count_service.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module Users + class AssignedIssuesCountService < ::BaseCountService + def initialize(current_user:, max_limit: User::MAX_LIMIT_FOR_ASSIGNEED_ISSUES_COUNT) + @current_user = current_user + @max_limit = max_limit + end + + def cache_key + ['users', @current_user.id, 'max_assigned_open_issues_count'] + end + + def cache_options + { force: false, expires_in: User::COUNT_CACHE_VALIDITY_PERIOD } + end + + # rubocop: disable CodeReuse/ActiveRecord + def uncached_count + # When a user has many assigned issues, counting them all can be very slow. + # As a workaround, we will short-circuit the counting query once the count reaches some threshold. + # + # Concretely, given a threshold, say 100 (= max_limit), + # iterate through the first 100 issues, sorted by ID desc, assigned to the user using `issue_assignees` table. + # For each issue iterated, use IssuesFinder to check if the issue should be counted. + initializer = IssueAssignee + .select(:issue_id).joins(", LATERAL (#{finder_constraint.to_sql}) as issues") + .where(user_id: @current_user.id) + .order(issue_id: :desc) + .limit(1) + recursive_finder = initializer.where("issue_assignees.issue_id < assigned_issues.issue_id") + + cte = <<~SQL + WITH RECURSIVE assigned_issues AS ( + ( + #{initializer.to_sql} + ) + UNION ALL + ( + SELECT next_assigned_issue.issue_id + FROM assigned_issues, + LATERAL ( + #{recursive_finder.to_sql} + ) next_assigned_issue + ) + ) SELECT COUNT(*) FROM (SELECT * FROM assigned_issues LIMIT #{@max_limit}) issues + SQL + + ApplicationRecord.connection.execute(cte).first["count"] + end + # rubocop: enable CodeReuse/ActiveRecord + + private + + # rubocop: disable CodeReuse/ActiveRecord + def finder_constraint + IssuesFinder.new(@current_user, assignee_id: @current_user.id, state: 'opened', non_archived: true) + .execute + .where("issues.id=issue_assignees.issue_id").limit(1) + end + # rubocop: enable CodeReuse/ActiveRecord + end +end diff --git a/app/services/users/banned_user_base_service.rb b/app/services/users/banned_user_base_service.rb index a582816283a..74c10581a6e 100644 --- a/app/services/users/banned_user_base_service.rb +++ b/app/services/users/banned_user_base_service.rb @@ -36,7 +36,7 @@ module Users end def log_event(user) - Gitlab::AppLogger.info(message: "User #{action}", user: "#{user.username}", email: "#{user.email}", "#{action}_by": "#{current_user.username}", ip_address: "#{current_user.current_sign_in_ip}") + Gitlab::AppLogger.info(message: "User #{action}", user: user.username.to_s, email: user.email.to_s, "#{action}_by": current_user.username.to_s, ip_address: current_user.current_sign_in_ip.to_s) end end end diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 8ef1b3e0613..064bf132d3d 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -117,7 +117,7 @@ module Users end def skip_user_confirmation_email_from_setting - !Gitlab::CurrentSettings.send_user_confirmation_email + Gitlab::CurrentSettings.email_confirmation_setting_off? end def use_fallback_name? diff --git a/app/services/users/keys_count_service.rb b/app/services/users/keys_count_service.rb index f82d27eded9..378093f2e1b 100644 --- a/app/services/users/keys_count_service.rb +++ b/app/services/users/keys_count_service.rb @@ -11,7 +11,7 @@ module Users end def relation_for_count - user.keys + user.keys.auth end def raw? diff --git a/app/services/users/migrate_records_to_ghost_user_service.rb b/app/services/users/migrate_records_to_ghost_user_service.rb index 2d92aaed7da..5d518803315 100644 --- a/app/services/users/migrate_records_to_ghost_user_service.rb +++ b/app/services/users/migrate_records_to_ghost_user_service.rb @@ -42,6 +42,7 @@ module Users migrate_award_emoji migrate_snippets migrate_reviews + migrate_releases end def post_migrate_records @@ -96,6 +97,10 @@ module Users batched_migrate(Review, :author_id) end + def migrate_releases + batched_migrate(Release, :author_id) + end + # rubocop:disable CodeReuse/ActiveRecord def batched_migrate(base_scope, column, batch_size: 50) loop do diff --git a/app/services/users/reject_service.rb b/app/services/users/reject_service.rb index 459dd81b74d..dc22b2ec21d 100644 --- a/app/services/users/reject_service.rb +++ b/app/services/users/reject_service.rb @@ -34,7 +34,7 @@ module Users end def log_event(user) - Gitlab::AppLogger.info(message: "User instance access request rejected", user: "#{user.username}", email: "#{user.email}", rejected_by: "#{current_user.username}", ip_address: "#{current_user.current_sign_in_ip}") + Gitlab::AppLogger.info(message: "User instance access request rejected", user: user.username.to_s, email: user.email.to_s, rejected_by: current_user.username.to_s, ip_address: current_user.current_sign_in_ip.to_s) end end end diff --git a/app/services/users/update_highest_member_role_service.rb b/app/services/users/update_highest_member_role_service.rb index 90a5966265d..fff001e04d7 100644 --- a/app/services/users/update_highest_member_role_service.rb +++ b/app/services/users/update_highest_member_role_service.rb @@ -17,9 +17,7 @@ module Users private def user_highest_role - @user_highest_role ||= begin - @user.user_highest_role || @user.build_user_highest_role - end + @user_highest_role ||= @user.user_highest_role || @user.build_user_highest_role end def highest_access_level diff --git a/app/services/web_hooks/log_execution_service.rb b/app/services/web_hooks/log_execution_service.rb index 448bb7d4097..b1da0c1642f 100644 --- a/app/services/web_hooks/log_execution_service.rb +++ b/app/services/web_hooks/log_execution_service.rb @@ -17,18 +17,32 @@ module WebHooks end def execute - update_hook_failure_state if WebHook.web_hooks_disable_failed?(hook) + update_hook_failure_state log_execution end private def log_execution + mask_response_headers + log_data[:request_headers]['X-Gitlab-Token'] = _('[REDACTED]') if hook.token? WebHookLog.create!(web_hook: hook, **log_data) end + def mask_response_headers + return unless hook.url_variables? + return unless log_data.key?(:response_headers) + + variables_map = hook.url_variables.invert.transform_values { "{#{_1}}" } + regex = Regexp.union(variables_map.keys) + + log_data[:response_headers].transform_values! do |value| + regex === value ? value.gsub(regex, variables_map) : value + end + end + # Perform this operation within an `Gitlab::ExclusiveLease` lock to make it # safe to be called concurrently from different workers. def update_hook_failure_state diff --git a/app/services/wiki_pages/update_service.rb b/app/services/wiki_pages/update_service.rb index 12b2cf87d5d..cf9eddbd13f 100644 --- a/app/services/wiki_pages/update_service.rb +++ b/app/services/wiki_pages/update_service.rb @@ -12,7 +12,7 @@ module WikiPages execute_hooks(page) ServiceResponse.success(payload: { page: page }) else - raise UpdateError, s_('Could not update wiki page') + raise UpdateError, _('Could not update wiki page') end rescue UpdateError, WikiPage::PageChangedError, WikiPage::PageRenameError => e page.update_attributes(@params) # rubocop:disable Rails/ActiveRecordAliases diff --git a/app/services/work_items/create_and_link_service.rb b/app/services/work_items/create_and_link_service.rb index 5cc358c4b4f..351ebc14564 100644 --- a/app/services/work_items/create_and_link_service.rb +++ b/app/services/work_items/create_and_link_service.rb @@ -6,7 +6,7 @@ module WorkItems # This class should always be run inside a transaction as we could end up with # new work items that were never associated with other work items as expected. class CreateAndLinkService - def initialize(project:, current_user: nil, params: {}, spam_params:, link_params: {}) + def initialize(project:, spam_params:, current_user: nil, params: {}, link_params: {}) @project = project @current_user = current_user @params = params diff --git a/app/services/work_items/create_from_task_service.rb b/app/services/work_items/create_from_task_service.rb index ef1d47c560d..ced5b17a21c 100644 --- a/app/services/work_items/create_from_task_service.rb +++ b/app/services/work_items/create_from_task_service.rb @@ -2,7 +2,7 @@ module WorkItems class CreateFromTaskService - def initialize(work_item:, current_user: nil, work_item_params: {}, spam_params:) + def initialize(work_item:, spam_params:, current_user: nil, work_item_params: {}) @work_item = work_item @current_user = current_user @work_item_params = work_item_params diff --git a/app/services/work_items/create_service.rb b/app/services/work_items/create_service.rb index 87cc690d666..c89ebc75b80 100644 --- a/app/services/work_items/create_service.rb +++ b/app/services/work_items/create_service.rb @@ -4,7 +4,7 @@ module WorkItems class CreateService < Issues::CreateService include WidgetableService - def initialize(project:, current_user: nil, params: {}, spam_params:, widget_params: {}) + def initialize(project:, spam_params:, current_user: nil, params: {}, widget_params: {}) super( project: project, current_user: current_user, diff --git a/app/services/work_items/delete_task_service.rb b/app/services/work_items/delete_task_service.rb index 3bb23576442..2a82a993b71 100644 --- a/app/services/work_items/delete_task_service.rb +++ b/app/services/work_items/delete_task_service.rb @@ -2,7 +2,7 @@ module WorkItems class DeleteTaskService - def initialize(work_item:, current_user: nil, task_params: {}, lock_version:) + def initialize(work_item:, lock_version:, current_user: nil, task_params: {}) @work_item = work_item @current_user = current_user @task_params = task_params |