diff options
Diffstat (limited to 'app/services')
74 files changed, 871 insertions, 357 deletions
diff --git a/app/services/alert_management/alerts/update_service.rb b/app/services/alert_management/alerts/update_service.rb index 7a9bcf2a52d..0769adc862e 100644 --- a/app/services/alert_management/alerts/update_service.rb +++ b/app/services/alert_management/alerts/update_service.rb @@ -12,6 +12,7 @@ module AlertManagement @alert = alert @param_errors = [] @status = params.delete(:status) + @status_change_reason = params.delete(:status_change_reason) super(project: alert.project, current_user: current_user, params: params) end @@ -36,7 +37,7 @@ module AlertManagement private - attr_reader :alert, :param_errors, :status + attr_reader :alert, :param_errors, :status, :status_change_reason def allowed? current_user&.can?(:update_alert_management_alert, alert) @@ -133,7 +134,7 @@ module AlertManagement end def add_status_change_system_note - SystemNoteService.change_alert_status(alert, current_user) + SystemNoteService.change_alert_status(alert, current_user, status_change_reason) end def resolve_todos @@ -144,13 +145,17 @@ module AlertManagement ::Issues::UpdateService.new( project: project, current_user: current_user, - params: { escalation_status: { status: status } } + params: { + escalation_status: { + status: status, + status_change_reason: " by changing the status of #{alert.to_reference(project)}" + } + } ).execute(alert.issue) end def should_sync_to_incident? - Feature.enabled?(:incident_escalations, project) && - alert.issue && + alert.issue && alert.issue.supports_escalation? && alert.issue.escalation_status && alert.issue.escalation_status.status != alert.status diff --git a/app/services/alert_management/create_alert_issue_service.rb b/app/services/alert_management/create_alert_issue_service.rb index a81c2380dad..ab8d1176b9e 100644 --- a/app/services/alert_management/create_alert_issue_service.rb +++ b/app/services/alert_management/create_alert_issue_service.rb @@ -22,8 +22,6 @@ module AlertManagement return result unless result.success? issue = result.payload[:issue] - return error(object_errors(alert), issue) unless associate_alert_with_issue(issue) - update_title_for(issue) SystemNoteService.new_alert_issue(alert, issue, user) @@ -47,14 +45,11 @@ module AlertManagement user, title: alert_presenter.title, description: alert_presenter.issue_description, - severity: alert.severity + severity: alert.severity, + alert: alert ).execute end - def associate_alert_with_issue(issue) - alert.update(issue_id: issue.id) - end - def update_title_for(issue) return unless issue.title == DEFAULT_ALERT_TITLE @@ -78,9 +73,5 @@ module AlertManagement alert.present end end - - def object_errors(object) - object.errors.full_messages.to_sentence - end end end diff --git a/app/services/audit_event_service.rb b/app/services/audit_event_service.rb index f2b1d89161c..ad733c455a9 100644 --- a/app/services/audit_event_service.rb +++ b/app/services/audit_event_service.rb @@ -5,7 +5,7 @@ class AuditEventService # Instantiates a new service # - # @param [User] author the user who authors the change + # @param [User, token String] author the entity who authors the change # @param [User, Project, Group] entity the scope which audit event belongs to # This param is also used to determine the visibility of the audit event. # - Project: events are visible at Project and Instance level @@ -44,7 +44,7 @@ class AuditEventService # Writes event to a file and creates an event record in DB # - # @return [AuditEvent] persited if saves and non-persisted if fails + # @return [AuditEvent] persisted if saves and non-persisted if fails def security_event log_security_event_to_file log_authentication_event_to_database diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index a92a2c8aef5..84518fd6b0e 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -14,6 +14,10 @@ module Auth :build_destroy_container_image ].freeze + FORBIDDEN_IMPORTING_SCOPES = %w[push delete *].freeze + + ActiveImportError = Class.new(StandardError) + def execute(authentication_abilities:) @authentication_abilities = authentication_abilities @@ -26,17 +30,27 @@ module Auth end { token: authorized_token(*scopes).encoded } + rescue ActiveImportError + error( + 'DENIED', + status: 403, + message: 'Your repository is currently being migrated to a new platform and writes are temporarily disabled. Go to https://gitlab.com/groups/gitlab-org/-/epics/5523 to learn more.' + ) end def self.full_access_token(*names) access_token(%w(*), names) end + def self.import_access_token + access_token(%w(*), ['import'], 'registry') + end + def self.pull_access_token(*names) access_token(['pull'], names) end - def self.access_token(actions, names) + def self.access_token(actions, names, type = 'repository') names = names.flatten registry = Gitlab.config.registry token = JSONWebToken::RSAToken.new(registry.key) @@ -46,10 +60,10 @@ module Auth token[:access] = names.map do |name| { - type: 'repository', + type: type, name: name, actions: actions, - migration_eligible: migration_eligible(repository_path: name) + migration_eligible: type == 'repository' ? migration_eligible(repository_path: name) : nil }.compact end @@ -104,6 +118,8 @@ module Auth def process_repository_access(type, path, actions) return unless path.valid? + raise ActiveImportError if actively_importing?(actions, path) + requested_project = path.repository_project return unless requested_project @@ -124,11 +140,19 @@ module Auth type: type, name: path.to_s, actions: authorized_actions, - migration_eligible: self.class.migration_eligible(project: requested_project), - cdn_redirect: cdn_redirect + migration_eligible: self.class.migration_eligible(project: requested_project) }.compact end + def actively_importing?(actions, path) + return false if FORBIDDEN_IMPORTING_SCOPES.intersection(actions).empty? + + container_repository = ContainerRepository.find_by_path(path) + return false unless container_repository + + container_repository.migration_importing? + end + def self.migration_eligible(project: nil, repository_path: nil) return unless Feature.enabled?(:container_registry_migration_phase1) @@ -151,13 +175,6 @@ module Auth false end - # This is used to determine whether blob download requests using a given JWT token should be redirected to Google - # Cloud CDN or not. The intent is to enable a percentage of time rollout for this new feature on the Container - # Registry side. See https://gitlab.com/gitlab-org/gitlab/-/issues/349417 for more details. - def cdn_redirect - Feature.enabled?(:container_registry_cdn_redirect) || nil - end - ## # Because we do not have two way communication with registry yet, # we create a container repository image resource when push to the diff --git a/app/services/boards/base_item_move_service.rb b/app/services/boards/base_item_move_service.rb index be16c595abb..9d711d83fd2 100644 --- a/app/services/boards/base_item_move_service.rb +++ b/app/services/boards/base_item_move_service.rb @@ -39,7 +39,7 @@ module Boards end def reposition_params(reposition_ids) - reposition_parent.merge(move_between_ids: reposition_ids) + { move_between_ids: reposition_ids } end def move_single_issuable(issuable, issuable_modification_params) @@ -91,7 +91,7 @@ module Boards end def move_between_ids(move_params) - ids = [move_params[:move_after_id], move_params[:move_before_id]] + ids = [move_params[:move_before_id], move_params[:move_after_id]] .map(&:to_i) .map { |m| m > 0 ? m : nil } diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index 959a7fa3ad2..90226b9d4e0 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -54,10 +54,6 @@ module Boards def update(issue, issue_modification_params) ::Issues::UpdateService.new(project: issue.project, current_user: current_user, params: issue_modification_params).execute(issue) end - - def reposition_parent - { board_group_id: board.group&.id } - end end end end diff --git a/app/services/branches/create_service.rb b/app/services/branches/create_service.rb index b5faf2ec281..7300b31e3b3 100644 --- a/app/services/branches/create_service.rb +++ b/app/services/branches/create_service.rb @@ -21,7 +21,7 @@ module Branches error("Failed to create branch '#{branch_name}': invalid reference name '#{ref}'") end rescue Gitlab::Git::PreReceiveError => e - Gitlab::ErrorTracking.track_exception(e, pre_receive_message: e.raw_message, branch_name: branch_name, ref: ref) + Gitlab::ErrorTracking.log_exception(e, pre_receive_message: e.raw_message, branch_name: branch_name, ref: ref) error(e.message) end diff --git a/app/services/ci/after_requeue_job_service.rb b/app/services/ci/after_requeue_job_service.rb index ee0ae6651ca..097b29cf143 100644 --- a/app/services/ci/after_requeue_job_service.rb +++ b/app/services/ci/after_requeue_job_service.rb @@ -22,13 +22,9 @@ module Ci end def dependent_jobs - if ::Feature.enabled?(:ci_order_subsequent_jobs_by_stage, @processable.pipeline.project, default_enabled: :yaml) - stage_dependent_jobs - .or(needs_dependent_jobs.except(:preload)) - .ordered_by_stage - else - stage_dependent_jobs | needs_dependent_jobs - end + stage_dependent_jobs + .or(needs_dependent_jobs.except(:preload)) + .ordered_by_stage end def process(job) diff --git a/app/services/ci/copy_cross_database_associations_service.rb b/app/services/ci/copy_cross_database_associations_service.rb new file mode 100644 index 00000000000..c69e966dc04 --- /dev/null +++ b/app/services/ci/copy_cross_database_associations_service.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Ci + class CopyCrossDatabaseAssociationsService + def execute(old_build, new_build) + ServiceResponse.success + end + end +end + +Ci::CopyCrossDatabaseAssociationsService.prepend_mod_with('Ci::CopyCrossDatabaseAssociationsService') diff --git a/app/services/ci/create_downstream_pipeline_service.rb b/app/services/ci/create_downstream_pipeline_service.rb index a65c22e273c..a2e53cbf9b8 100644 --- a/app/services/ci/create_downstream_pipeline_service.rb +++ b/app/services/ci/create_downstream_pipeline_service.rb @@ -125,7 +125,9 @@ module Ci config_checksum(pipeline) unless pipeline.child? end - pipeline_checksums.uniq.length != pipeline_checksums.length + # To avoid false positives we allow 1 cycle in the ancestry and + # fail when 2 cycles are detected: A -> B -> A -> B -> A + pipeline_checksums.tally.any? { |_checksum, occurrences| occurrences > 2 } end end @@ -137,7 +139,7 @@ module Ci end def config_checksum(pipeline) - [pipeline.project_id, pipeline.ref].hash + [pipeline.project_id, pipeline.ref, pipeline.source].hash end end end diff --git a/app/services/ci/pipeline_schedule_service.rb b/app/services/ci/pipeline_schedule_service.rb index 596c3b80bda..536eaa56f9b 100644 --- a/app/services/ci/pipeline_schedule_service.rb +++ b/app/services/ci/pipeline_schedule_service.rb @@ -3,6 +3,8 @@ module Ci class PipelineScheduleService < BaseService def execute(schedule) + return unless project.persisted? + # 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! diff --git a/app/services/ci/process_sync_events_service.rb b/app/services/ci/process_sync_events_service.rb index 11ce6e8eeaf..d90ee02b1c6 100644 --- a/app/services/ci/process_sync_events_service.rb +++ b/app/services/ci/process_sync_events_service.rb @@ -2,7 +2,6 @@ module Ci class ProcessSyncEventsService - include Gitlab::Utils::StrongMemoize include ExclusiveLeaseGuard BATCH_SIZE = 1000 @@ -10,22 +9,27 @@ module Ci def initialize(sync_event_class, sync_class) @sync_event_class = sync_event_class @sync_class = sync_class + @results = {} end def execute - return unless ::Feature.enabled?(:ci_namespace_project_mirrors, default_enabled: :yaml) - # preventing parallel processing over the same event table try_obtain_lease { process_events } enqueue_worker_if_there_still_event + + @results end private def process_events + add_result(estimated_total_events: @sync_event_class.upper_bound_count) + events = @sync_event_class.preload_synced_relation.first(BATCH_SIZE) + add_result(consumable_events: events.size) + return if events.empty? processed_events = [] @@ -37,6 +41,7 @@ module Ci processed_events << event end ensure + add_result(processed_events: processed_events.size) @sync_event_class.id_in(processed_events).delete_all end end @@ -52,5 +57,9 @@ module Ci def lease_timeout 1.minute end + + def add_result(result) + @results.merge!(result) + end end end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index e0f0f8f58b8..59c4c17a964 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -159,7 +159,7 @@ module Ci return end - if runner.can_pick?(build) + if runner.matches_build?(build) @metrics.increment_queue_operation(:build_can_pick) else @metrics.increment_queue_operation(:build_not_pick) diff --git a/app/services/ci/register_runner_service.rb b/app/services/ci/register_runner_service.rb index 0a2027e33ce..7c6cd82565d 100644 --- a/app/services/ci/register_runner_service.rb +++ b/app/services/ci/register_runner_service.rb @@ -3,7 +3,7 @@ module Ci class RegisterRunnerService def execute(registration_token, attributes) - runner_type_attrs = check_token_and_extract_attrs(registration_token) + runner_type_attrs = extract_runner_type_attrs(registration_token) return unless runner_type_attrs @@ -12,16 +12,32 @@ module Ci private - def check_token_and_extract_attrs(registration_token) + def extract_runner_type_attrs(registration_token) + @attrs_from_token ||= check_token(registration_token) + + return unless @attrs_from_token + + attrs = @attrs_from_token.clone + case attrs[:runner_type] + when :project_type + attrs[:projects] = [attrs.delete(:scope)] + when :group_type + attrs[:groups] = [attrs.delete(:scope)] + end + + attrs + end + + def check_token(registration_token) if runner_registration_token_valid?(registration_token) # Create shared runner. Requires admin access { runner_type: :instance_type } elsif runner_registrar_valid?('project') && project = ::Project.find_by_runners_token(registration_token) # Create a specific runner for the project - { runner_type: :project_type, projects: [project] } + { runner_type: :project_type, scope: project } elsif runner_registrar_valid?('group') && group = ::Group.find_by_runners_token(registration_token) # Create a specific runner for the group - { runner_type: :group_type, groups: [group] } + { runner_type: :group_type, scope: group } end end @@ -32,5 +48,11 @@ module Ci def runner_registrar_valid?(type) Feature.disabled?(:runner_registration_control) || Gitlab::CurrentSettings.valid_runner_registrars.include?(type) end + + def token_scope + @attrs_from_token[:scope] + end end end + +Ci::RegisterRunnerService.prepend_mod diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index 7e5d5373648..73c5d0163da 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -40,17 +40,15 @@ module Ci new_build = clone_build(build) new_build.run_after_commit do + ::Ci::CopyCrossDatabaseAssociationsService.new.execute(build, new_build) + + ::Deployments::CreateForBuildService.new.execute(new_build) + ::MergeRequests::AddTodoWhenBuildFailsService .new(project: project) .close(new_build) end - if create_deployment_in_separate_transaction? - new_build.run_after_commit do |new_build| - ::Deployments::CreateForBuildService.new.execute(new_build) - end - end - ::Ci::Pipelines::AddJobService.new(build.pipeline).execute!(new_build) do |job| BulkInsertableAssociations.with_bulk_insert do job.save! @@ -74,11 +72,7 @@ module Ci def check_assignable_runners!(build); end def clone_build(build) - project.builds.new(build_attributes(build)).tap do |new_build| - unless create_deployment_in_separate_transaction? - new_build.assign_attributes(deployment_attributes_for(new_build, build)) - end - end + project.builds.new(build_attributes(build)) end def build_attributes(build) @@ -86,7 +80,7 @@ module Ci [attribute, build.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend end - if create_deployment_in_separate_transaction? && build.persisted_environment.present? + if build.persisted_environment.present? attributes[:metadata_attributes] ||= {} attributes[:metadata_attributes][:expanded_environment_name] = build.expanded_environment_name end @@ -94,17 +88,6 @@ module Ci attributes[:user] = current_user attributes end - - def deployment_attributes_for(new_build, old_build) - ::Gitlab::Ci::Pipeline::Seed::Build - .deployment_attributes_for(new_build, old_build.persisted_environment) - end - - def create_deployment_in_separate_transaction? - strong_memoize(:create_deployment_in_separate_transaction) do - ::Feature.enabled?(:create_deployment_in_separate_transaction, project, default_enabled: :yaml) - end - end end end diff --git a/app/services/ci/unregister_runner_service.rb b/app/services/ci/unregister_runner_service.rb new file mode 100644 index 00000000000..97d9852b7ed --- /dev/null +++ b/app/services/ci/unregister_runner_service.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Ci + class UnregisterRunnerService + attr_reader :runner + + # @param [Ci::Runner] runner the runner to unregister/destroy + def initialize(runner) + @runner = runner + end + + def execute + @runner&.destroy + end + end +end diff --git a/app/services/ci/update_build_queue_service.rb b/app/services/ci/update_build_queue_service.rb index 2e38969c7a9..5a011a8cac6 100644 --- a/app/services/ci/update_build_queue_service.rb +++ b/app/services/ci/update_build_queue_service.rb @@ -100,7 +100,7 @@ module Ci def tick_for(build, runners) runners = runners.with_recent_runner_queue - runners = runners.with_tags if Feature.enabled?(:ci_preload_runner_tags, default_enabled: :yaml) + runners = runners.with_tags metrics.observe_active_runners(-> { runners.to_a.size }) diff --git a/app/services/ci/update_runner_service.rb b/app/services/ci/update_runner_service.rb index e4117a51fe6..4a17e25c0cc 100644 --- a/app/services/ci/update_runner_service.rb +++ b/app/services/ci/update_runner_service.rb @@ -9,6 +9,8 @@ module Ci end def update(params) + params[:active] = !params.delete(:paused) if params.include?(:paused) + runner.update(params).tap do |updated| runner.tick_runner_queue if updated end diff --git a/app/services/concerns/members/bulk_create_users.rb b/app/services/concerns/members/bulk_create_users.rb index b98917f1396..9cfef96311e 100644 --- a/app/services/concerns/members/bulk_create_users.rb +++ b/app/services/concerns/members/bulk_create_users.rb @@ -48,6 +48,7 @@ module Members end if user_ids.present? + # we should handle the idea of existing members where users are passed as users - https://gitlab.com/gitlab-org/gitlab/-/issues/352617 # the below will automatically discard invalid user_ids users.concat(User.id_in(user_ids)) # helps not have to perform another query per user id to see if the member exists later on when fetching diff --git a/app/services/concerns/rate_limited_service.rb b/app/services/concerns/rate_limited_service.rb index 87cba7814fe..c8dc60355cf 100644 --- a/app/services/concerns/rate_limited_service.rb +++ b/app/services/concerns/rate_limited_service.rb @@ -17,7 +17,7 @@ module RateLimitedService end def log_request(request, current_user) - rate_limiter.class.log_request(request, "#{key}_request_limit".to_sym, current_user) + rate_limiter.log_request(request, "#{key}_request_limit".to_sym, current_user) end private @@ -26,20 +26,19 @@ module RateLimitedService end class RateLimiterScopedAndKeyed - attr_reader :key, :opts, :rate_limiter_klass + attr_reader :key, :opts, :rate_limiter - def initialize(key:, opts:, rate_limiter_klass:) + def initialize(key:, opts:, rate_limiter:) @key = key @opts = opts - @rate_limiter_klass = rate_limiter_klass + @rate_limiter = rate_limiter end def rate_limit!(service) evaluated_scope = evaluated_scope_for(service) return if feature_flag_disabled?(evaluated_scope[:project]) - rate_limiter = new_rate_limiter(evaluated_scope) - if rate_limiter.throttled? + if rate_limiter.throttled?(key, **opts.merge(scope: evaluated_scope.values, users_allowlist: users_allowlist)) raise RateLimitedError.new(key: key, rate_limiter: rate_limiter), _('This endpoint has been requested too many times. Try again later.') end end @@ -59,20 +58,16 @@ module RateLimitedService def feature_flag_disabled?(project) Feature.disabled?("rate_limited_service_#{key}", project, default_enabled: :yaml) end - - def new_rate_limiter(evaluated_scope) - rate_limiter_klass.new(key, **opts.merge(scope: evaluated_scope.values, users_allowlist: users_allowlist)) - end end prepended do attr_accessor :rate_limiter_bypassed cattr_accessor :rate_limiter_scoped_and_keyed - def self.rate_limit(key:, opts:, rate_limiter_klass: ::Gitlab::ApplicationRateLimiter) + def self.rate_limit(key:, opts:, rate_limiter: ::Gitlab::ApplicationRateLimiter) self.rate_limiter_scoped_and_keyed = RateLimiterScopedAndKeyed.new(key: key, opts: opts, - rate_limiter_klass: rate_limiter_klass) + rate_limiter: rate_limiter) end end diff --git a/app/services/google_cloud/create_service_accounts_service.rb b/app/services/google_cloud/create_service_accounts_service.rb index fa025e8f672..e360b3a8e4e 100644 --- a/app/services/google_cloud/create_service_accounts_service.rb +++ b/app/services/google_cloud/create_service_accounts_service.rb @@ -5,6 +5,7 @@ module GoogleCloud def execute service_account = google_api_client.create_service_account(gcp_project_id, service_account_name, service_account_desc) service_account_key = google_api_client.create_service_account_key(gcp_project_id, service_account.unique_id) + google_api_client.grant_service_account_roles(gcp_project_id, service_account.email) service_accounts_service.add_for_project( environment_name, @@ -35,7 +36,7 @@ module GoogleCloud end def google_api_client - GoogleApi::CloudPlatform::Client.new(google_oauth2_token, nil) + @google_api_client_instance ||= GoogleApi::CloudPlatform::Client.new(google_oauth2_token, nil) end def service_accounts_service @@ -50,7 +51,7 @@ module GoogleCloud "GitLab generated service account for project '#{project.name}' and environment '#{environment_name}'" end - # Overriden in EE + # Overridden in EE def environment_protected? false end diff --git a/app/services/google_cloud/enable_cloud_run_service.rb b/app/services/google_cloud/enable_cloud_run_service.rb new file mode 100644 index 00000000000..643f2b2b6d2 --- /dev/null +++ b/app/services/google_cloud/enable_cloud_run_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module GoogleCloud + class EnableCloudRunService < :: BaseService + def execute + gcp_project_ids = unique_gcp_project_ids + + if gcp_project_ids.empty? + error("No GCP projects found. Configure a service account or GCP_PROJECT_ID ci variable.") + else + google_api_client = GoogleApi::CloudPlatform::Client.new(token_in_session, nil) + + gcp_project_ids.each do |gcp_project_id| + google_api_client.enable_cloud_run(gcp_project_id) + google_api_client.enable_artifacts_registry(gcp_project_id) + google_api_client.enable_cloud_build(gcp_project_id) + end + + success({ gcp_project_ids: gcp_project_ids }) + end + end + + private + + def unique_gcp_project_ids + all_gcp_project_ids = project.variables.filter { |var| var.key == 'GCP_PROJECT_ID' }.map { |var| var.value } + all_gcp_project_ids.uniq + end + + def token_in_session + @params[:token_in_session] + end + end +end diff --git a/app/services/google_cloud/generate_pipeline_service.rb b/app/services/google_cloud/generate_pipeline_service.rb new file mode 100644 index 00000000000..077f815e60c --- /dev/null +++ b/app/services/google_cloud/generate_pipeline_service.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +module GoogleCloud + class GeneratePipelineService < :: BaseService + ACTION_DEPLOY_TO_CLOUD_RUN = 'DEPLOY_TO_CLOUD_RUN' + ACTION_DEPLOY_TO_CLOUD_STORAGE = 'DEPLOY_TO_CLOUD_STORAGE' + + def execute + commit_attributes = generate_commit_attributes + create_branch_response = ::Branches::CreateService.new(project, current_user) + .execute(commit_attributes[:branch_name], project.default_branch) + + if create_branch_response[:status] == :error + return create_branch_response + end + + branch = create_branch_response[:branch] + + service = default_branch_gitlab_ci_yml.present? ? ::Files::UpdateService : ::Files::CreateService + + commit_response = service.new(project, current_user, commit_attributes).execute + + if commit_response[:status] == :error + return commit_response + end + + success({ branch_name: branch.name, commit: commit_response }) + end + + private + + def action + @params[:action] + end + + def generate_commit_attributes + if action == ACTION_DEPLOY_TO_CLOUD_RUN + branch_name = "deploy-to-cloud-run-#{SecureRandom.hex(8)}" + { + commit_message: 'Enable Cloud Run deployments', + file_path: '.gitlab-ci.yml', + file_content: pipeline_content('gcp/cloud-run.gitlab-ci.yml'), + branch_name: branch_name, + start_branch: branch_name + } + elsif action == ACTION_DEPLOY_TO_CLOUD_STORAGE + branch_name = "deploy-to-cloud-storage-#{SecureRandom.hex(8)}" + { + commit_message: 'Enable Cloud Storage deployments', + file_path: '.gitlab-ci.yml', + file_content: pipeline_content('gcp/cloud-storage.gitlab-ci.yml'), + branch_name: branch_name, + start_branch: branch_name + } + end + end + + def default_branch_gitlab_ci_yml + @default_branch_gitlab_ci_yml ||= project.repository.gitlab_ci_yml_for(project.default_branch) + end + + def pipeline_content(include_path) + gitlab_ci_yml = Gitlab::Config::Loader::Yaml.new(default_branch_gitlab_ci_yml || '{}').load! + append_remote_include(gitlab_ci_yml, "https://gitlab.com/gitlab-org/incubation-engineering/five-minute-production/library/-/raw/main/#{include_path}") + end + + def append_remote_include(gitlab_ci_yml, include_url) + stages = gitlab_ci_yml['stages'] || [] + gitlab_ci_yml['stages'] = (stages + %w[build test deploy]).uniq + + includes = gitlab_ci_yml['include'] || [] + includes = Array.wrap(includes) + includes << { 'remote' => include_url } + gitlab_ci_yml['include'] = includes.uniq + + gitlab_ci_yml.to_yaml + end + end +end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index da3cebc2e6d..67cbbaf84f6 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -61,6 +61,8 @@ module Groups delay = Namespaces::InviteTeamEmailService::DELIVERY_DELAY_IN_MINUTES Namespaces::InviteTeamEmailWorker.perform_in(delay, group.id, current_user.id) end + + track_experiment_event end def remove_unallowed_params @@ -112,6 +114,15 @@ module Groups @group.shared_runners_enabled = @group.parent.shared_runners_enabled @group.allow_descendants_override_disabled_shared_runners = @group.parent.allow_descendants_override_disabled_shared_runners end + + def track_experiment_event + return unless group.persisted? + + # Track namespace created events to relate them with signed up events for + # the same experiment. This will let us associate created namespaces to + # users that signed up from the experimental logged out header. + experiment(:logged_out_marketing_header, actor: current_user).track(:namespace_created, namespace: group) + end end end diff --git a/app/services/groups/update_statistics_service.rb b/app/services/groups/update_statistics_service.rb new file mode 100644 index 00000000000..9efce79ef42 --- /dev/null +++ b/app/services/groups/update_statistics_service.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Groups + class UpdateStatisticsService + attr_reader :group, :statistics + + def initialize(group, statistics: []) + @group = group + @statistics = statistics + end + + def execute + unless group + return ServiceResponse.error(message: 'Invalid group', http_status: 400) + end + + namespace_statistics.refresh!(only: statistics.map(&:to_sym)) + + ServiceResponse.success(message: 'Group statistics successfully updated.') + end + + private + + def namespace_statistics + @namespace_statistics ||= group.namespace_statistics || group.build_namespace_statistics + end + end +end diff --git a/app/services/incident_management/create_incident_label_service.rb b/app/services/incident_management/create_incident_label_service.rb deleted file mode 100644 index 595f5df184f..00000000000 --- a/app/services/incident_management/create_incident_label_service.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -module IncidentManagement - class CreateIncidentLabelService < BaseService - LABEL_PROPERTIES = { - title: 'incident', - color: '#CC0033', - description: <<~DESCRIPTION.chomp - Denotes a disruption to IT services and \ - the associated issues require immediate attention - DESCRIPTION - }.freeze - - def execute - label = Labels::FindOrCreateService - .new(current_user, project, **LABEL_PROPERTIES) - .execute(skip_authorization: true) - - ServiceResponse.success(payload: { label: label }) - end - end -end diff --git a/app/services/incident_management/incidents/create_service.rb b/app/services/incident_management/incidents/create_service.rb index f8437290d9b..ef66325fdcc 100644 --- a/app/services/incident_management/incidents/create_service.rb +++ b/app/services/incident_management/incidents/create_service.rb @@ -2,15 +2,16 @@ module IncidentManagement module Incidents - class CreateService < BaseService + class CreateService < ::BaseProjectService ISSUE_TYPE = 'incident' - def initialize(project, current_user, title:, description:, severity: IssuableSeverity::DEFAULT) - super(project, current_user) + def initialize(project, current_user, title:, description:, severity: IssuableSeverity::DEFAULT, alert: nil) + super(project: project, current_user: current_user) @title = title @description = description @severity = severity + @alert = alert end def execute @@ -21,11 +22,16 @@ module IncidentManagement title: title, description: description, issue_type: ISSUE_TYPE, - severity: severity + severity: severity, + alert_management_alert: alert }, spam_params: nil ).execute + if alert + return error(alert.errors.full_messages.to_sentence, issue) unless alert.valid? + end + return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid? success(issue) @@ -33,7 +39,7 @@ module IncidentManagement private - attr_reader :title, :description, :severity + attr_reader :title, :description, :severity, :alert def success(issue) ServiceResponse.success(payload: { issue: issue }) diff --git a/app/services/incident_management/issuable_escalation_statuses/after_update_service.rb b/app/services/incident_management/issuable_escalation_statuses/after_update_service.rb index a7a99f88b32..b7f8b268f18 100644 --- a/app/services/incident_management/issuable_escalation_statuses/after_update_service.rb +++ b/app/services/incident_management/issuable_escalation_statuses/after_update_service.rb @@ -3,12 +3,12 @@ module IncidentManagement module IssuableEscalationStatuses class AfterUpdateService < ::BaseProjectService - def initialize(issuable, current_user) + def initialize(issuable, current_user, **params) @issuable = issuable @escalation_status = issuable.escalation_status @alert = issuable.alert_management_alert - super(project: issuable.project, current_user: current_user) + super(project: issuable.project, current_user: current_user, params: params) end def execute @@ -22,19 +22,27 @@ module IncidentManagement attr_reader :issuable, :escalation_status, :alert def after_update - sync_to_alert + sync_status_to_alert + add_status_system_note end - def sync_to_alert + def sync_status_to_alert return unless alert return if alert.status == escalation_status.status ::AlertManagement::Alerts::UpdateService.new( alert, current_user, - status: escalation_status.status_name + status: escalation_status.status_name, + status_change_reason: " by changing the incident status of #{issuable.to_reference(project)}" ).execute end + + def add_status_system_note + return unless escalation_status.status_previously_changed? + + SystemNoteService.change_incident_status(issuable, current_user, params[:status_change_reason]) + end end end end diff --git a/app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb b/app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb index 1a660e1a163..8f591b375ee 100644 --- a/app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb +++ b/app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb @@ -2,18 +2,16 @@ module IncidentManagement module IssuableEscalationStatuses - class PrepareUpdateService + class PrepareUpdateService < ::BaseProjectService include Gitlab::Utils::StrongMemoize - SUPPORTED_PARAMS = %i[status].freeze - - InvalidParamError = Class.new(StandardError) + SUPPORTED_PARAMS = %i[status status_change_reason].freeze def initialize(issuable, current_user, params) @issuable = issuable - @current_user = current_user - @params = params.dup || {} - @project = issuable.project + @param_errors = [] + + super(project: issuable.project, current_user: current_user, params: Hash(params)) end def execute @@ -23,19 +21,18 @@ module IncidentManagement filter_attributes filter_redundant_params + return invalid_param_error if param_errors.any? + ServiceResponse.success(payload: { escalation_status: params }) - rescue InvalidParamError - invalid_param_error end private - attr_reader :issuable, :current_user, :params, :project + attr_reader :issuable, :param_errors def available? - Feature.enabled?(:incident_escalations, project) && + issuable.supports_escalation? && user_has_permissions? && - issuable.supports_escalation? && escalation_status.present? end @@ -66,7 +63,7 @@ module IncidentManagement return unless status status_event = escalation_status.status_event_for(status) - raise InvalidParamError unless status_event + add_param_error(:status) && return unless status_event params[:status_event] = status_event end @@ -85,12 +82,16 @@ module IncidentManagement end end + def add_param_error(param) + param_errors << param + end + def availability_error ServiceResponse.error(message: 'Escalation status updates are not available for this issue, user, or project.') end def invalid_param_error - ServiceResponse.error(message: 'Invalid value was provided for a parameter.') + ServiceResponse.error(message: "Invalid value was provided for parameters: #{param_errors.join(', ')}") end end end diff --git a/app/services/issuable/common_system_notes_service.rb b/app/services/issuable/common_system_notes_service.rb index 38050708fc5..9ee54c7ba0f 100644 --- a/app/services/issuable/common_system_notes_service.rb +++ b/app/services/issuable/common_system_notes_service.rb @@ -71,7 +71,7 @@ module Issuable def create_title_change_note(old_title) create_draft_note(old_title) - if issuable.wipless_title_changed(old_title) + if issuable.draftless_title_changed(old_title) SystemNoteService.change_title(issuable, issuable.project, current_user, old_title) end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index ecf10cf97a8..95093b88155 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -162,6 +162,8 @@ class IssuableBaseService < ::BaseProjectService return unless result.success? && result.payload.present? + @escalation_status_change_reason = result[:escalation_status].delete(:status_change_reason) + params[:incident_management_issuable_escalation_status_attributes] = result[:escalation_status] end @@ -492,11 +494,12 @@ class IssuableBaseService < ::BaseProjectService def handle_move_between_ids(issuable_position) return unless params[:move_between_ids] - after_id, before_id = params.delete(:move_between_ids) - positioning_scope_id = params.delete(positioning_scope_key) + before_id, after_id = params.delete(:move_between_ids) + + positioning_scope = issuable_position.class.relative_positioning_query_base(issuable_position) - issuable_before = issuable_for_positioning(before_id, positioning_scope_id) - issuable_after = issuable_for_positioning(after_id, positioning_scope_id) + issuable_before = issuable_for_positioning(before_id, positioning_scope) + issuable_after = issuable_for_positioning(after_id, positioning_scope) raise ActiveRecord::RecordNotFound unless issuable_before || issuable_after @@ -521,7 +524,7 @@ class IssuableBaseService < ::BaseProjectService def invalidate_cache_counts(issuable, users: []) users.each do |user| - user.public_send("invalidate_#{issuable.model_name.singular}_cache_counts") # rubocop:disable GitlabSecurity/PublicSend + user.public_send("invalidate_#{issuable.noteable_target_type_name}_cache_counts") # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 37d667d4be8..61a95e49228 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -2,6 +2,7 @@ module Issues class BaseService < ::IssuableBaseService + extend ::Gitlab::Utils::Override include IncidentManagement::UsageData include IssueTypeHelpers @@ -61,6 +62,21 @@ module Issues issue.system_note_timestamp = params[:created_at] || params[:updated_at] end + override :handle_move_between_ids + def handle_move_between_ids(issue) + issue.check_repositioning_allowed! if params[:move_between_ids] + + super + + rebalance_if_needed(issue) + end + + def issuable_for_positioning(id, positioning_scope) + return unless id + + positioning_scope.find(id) + end + def create_assignee_note(issue, old_assignees) SystemNoteService.change_issuable_assignees( issue, issue.project, current_user, old_assignees) diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 65f143d0b21..ff45091c7e6 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -81,7 +81,7 @@ module Issues return if alert.resolved? if alert.resolve - SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: current_user).closed_alert_issue(issue) + SystemNoteService.change_alert_status(alert, current_user, " by closing incident #{issue.to_reference(project)}") else Gitlab::AppLogger.warn( message: 'Cannot resolve an associated Alert Management alert', @@ -97,7 +97,7 @@ module Issues status = issue.incident_management_issuable_escalation_status || issue.build_incident_management_issuable_escalation_status - SystemNoteService.resolve_incident_status(issue, current_user) if status.resolve + SystemNoteService.change_incident_status(issue, current_user, ' by closing the incident') if status.resolve end def store_first_mentioned_in_commit_at(issue, merge_request, max_commit_lookup: 100) diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index e29bcf4a453..7fbf7c6af58 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -21,6 +21,8 @@ module Issues def execute(skip_system_notes: false) @issue = @build_service.execute + handle_move_between_ids(@issue) + filter_resolve_discussion_params create(@issue, skip_system_notes: skip_system_notes) diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index 4418b4eb2bf..e210e6a2362 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -2,6 +2,8 @@ module Issues class MoveService < Issuable::Clone::BaseService + extend ::Gitlab::Utils::Override + MoveError = Class.new(StandardError) def execute(issue, target_project) @@ -47,6 +49,7 @@ module Issues .sent_notifications.update_all(project_id: new_entity.project_id, noteable_id: new_entity.id) end + override :update_old_entity def update_old_entity super @@ -54,6 +57,13 @@ module Issues mark_as_moved end + override :update_new_entity + def update_new_entity + super + + copy_contacts + end + def create_new_entity new_params = { id: nil, @@ -99,6 +109,13 @@ module Issues target_issue_links.update_all(target_id: new_entity.id) end + def copy_contacts + return unless Feature.enabled?(:customer_relations, original_entity.project.root_ancestor) + return unless original_entity.project.root_ancestor == new_entity.project.root_ancestor + + new_entity.customer_relations_contacts = original_entity.customer_relations_contacts + end + def notify_participants notification_service.async.issue_moved(original_entity, new_entity, @current_user) end diff --git a/app/services/issues/reorder_service.rb b/app/services/issues/reorder_service.rb index 9c5fbec7d8e..5443d41ac30 100644 --- a/app/services/issues/reorder_service.rb +++ b/app/services/issues/reorder_service.rb @@ -2,47 +2,31 @@ module Issues class ReorderService < Issues::BaseService + include Gitlab::Utils::StrongMemoize + def execute(issue) return false unless can?(current_user, :update_issue, issue) - return false if group && !can?(current_user, :read_group, group) - - attrs = issue_params(group) - return false if attrs.empty? + return false unless move_between_ids - update(issue, attrs) + update(issue, { move_between_ids: move_between_ids }) end private - def group - return unless params[:group_full_path] - - @group ||= Group.find_by_full_path(params[:group_full_path]) - end - def update(issue, attrs) ::Issues::UpdateService.new(project: project, current_user: current_user, params: attrs).execute(issue) rescue ActiveRecord::RecordNotFound false end - def issue_params(group) - attrs = {} - - if move_between_ids - attrs[:move_between_ids] = move_between_ids - attrs[:board_group_id] = group&.id - end - - attrs - end - def move_between_ids - ids = [params[:move_after_id], params[:move_before_id]] - .map(&:to_i) - .map { |m| m > 0 ? m : nil } + strong_memoize(:move_between_ids) do + ids = [params[:move_before_id], params[:move_after_id]] + .map(&:to_i) + .map { |m| m > 0 ? m : nil } - ids.any? ? ids : nil + ids.any? ? ids : nil + end end end end diff --git a/app/services/issues/set_crm_contacts_service.rb b/app/services/issues/set_crm_contacts_service.rb index 947d46f0809..2edc944435b 100644 --- a/app/services/issues/set_crm_contacts_service.rb +++ b/app/services/issues/set_crm_contacts_service.rb @@ -16,6 +16,9 @@ module Issues determine_changes if params[:replace_ids].present? return error_too_many if too_many? + @added_count = 0 + @removed_count = 0 + add if params[:add_ids].present? remove if params[:remove_ids].present? @@ -25,6 +28,7 @@ module Issues if issue.valid? GraphqlTriggers.issue_crm_contacts_updated(issue) issue.touch + create_system_note ServiceResponse.success(payload: issue) else # The default error isn't very helpful: "Issue customer relations contacts is invalid" @@ -36,7 +40,7 @@ module Issues private - attr_accessor :issue, :errors, :existing_ids + attr_accessor :issue, :errors, :existing_ids, :added_count, :removed_count def determine_changes params[:add_ids] = params[:replace_ids] - existing_ids @@ -48,16 +52,24 @@ module Issues end def add_by_email - contact_ids = ::CustomerRelations::Contact.find_ids_by_emails(project_group, params[:add_emails]) + contact_ids = ::CustomerRelations::Contact.find_ids_by_emails(project_group, emails(:add_emails)) add_by_id(contact_ids) end + def emails(key) + params[key].map do |email| + extract_email_from_request_param(email) + end + end + def add_by_id(contact_ids) contact_ids -= existing_ids contact_ids.uniq.each do |contact_id| issue_contact = issue.issue_customer_relations_contacts.create(contact_id: contact_id) - unless issue_contact.persisted? + if issue_contact.persisted? + @added_count += 1 + else # The validation ensures that the id exists and the user has permission errors << "#{contact_id}: The resource that you are attempting to access does not exist or you don't have permission to perform this action" end @@ -69,17 +81,24 @@ module Issues end def remove_by_email - contact_ids = ::CustomerRelations::IssueContact.find_contact_ids_by_emails(issue.id, params[:remove_emails]) + contact_ids = ::CustomerRelations::IssueContact.find_contact_ids_by_emails(issue.id, emails(:remove_emails)) remove_by_id(contact_ids) end def remove_by_id(contact_ids) contact_ids &= existing_ids - issue.issue_customer_relations_contacts + @removed_count += issue.issue_customer_relations_contacts .where(contact_id: contact_ids) # rubocop: disable CodeReuse/ActiveRecord .delete_all end + def extract_email_from_request_param(email_param) + email_param.delete_prefix(::CustomerRelations::Contact.reference_prefix_quoted) + .delete_prefix(::CustomerRelations::Contact.reference_prefix) + .delete_suffix(::CustomerRelations::Contact.reference_postfix) + .tr('"', '') + end + def allowed? current_user&.can?(:set_issue_crm_contacts, issue) end @@ -116,6 +135,11 @@ module Issues params[:add_emails] && params[:add_emails].length > MAX_ADDITIONAL_CONTACTS end + def create_system_note + SystemNoteService.change_issuable_contacts( + issue, issue.project, current_user, added_count, removed_count) + end + def error_no_permissions ServiceResponse.error(message: _('You have insufficient permissions to set customer relations contacts for this issue')) end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index aecb22453b7..8372cd919e5 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -2,8 +2,6 @@ module Issues class UpdateService < Issues::BaseService - extend ::Gitlab::Utils::Override - # NOTE: For Issues::UpdateService, we default the spam_params to nil, because spam_checking is not # necessary in many cases, and we don't want to require every caller to explicitly pass it as nil # to disable spam checking. @@ -92,18 +90,6 @@ module Issues todo_service.update_issue(issuable, current_user) end - def handle_move_between_ids(issue) - issue.check_repositioning_allowed! if params[:move_between_ids] - - super - - rebalance_if_needed(issue) - end - - def positioning_scope_key - :board_group_id - end - # rubocop: disable CodeReuse/ActiveRecord def change_issue_duplicate(issue) canonical_issue_id = params.delete(:canonical_issue_id) @@ -214,23 +200,12 @@ module Issues return unless old_escalation_status.present? return if issue.escalation_status&.slice(:status, :policy_id) == old_escalation_status - ::IncidentManagement::IssuableEscalationStatuses::AfterUpdateService.new(issue, current_user).execute - end - - # rubocop: disable CodeReuse/ActiveRecord - def issuable_for_positioning(id, board_group_id = nil) - return unless id - - issue = - if board_group_id - IssuesFinder.new(current_user, group_id: board_group_id, include_subgroups: true).find_by(id: id) - else - project.issues.find(id) - end - - issue if can?(current_user, :update_issue, issue) + ::IncidentManagement::IssuableEscalationStatuses::AfterUpdateService.new( + issue, + current_user, + status_change_reason: @escalation_status_change_reason # Defined in IssuableBaseService before save + ).execute end - # rubocop: enable CodeReuse/ActiveRecord def create_confidentiality_note(issue) SystemNoteService.change_issue_confidentiality(issue, issue.project, current_user) diff --git a/app/services/loose_foreign_keys/batch_cleaner_service.rb b/app/services/loose_foreign_keys/batch_cleaner_service.rb index de52cbba576..f3db2037911 100644 --- a/app/services/loose_foreign_keys/batch_cleaner_service.rb +++ b/app/services/loose_foreign_keys/batch_cleaner_service.rb @@ -2,6 +2,9 @@ module LooseForeignKeys class BatchCleanerService + CLEANUP_ATTEMPTS_BEFORE_RESCHEDULE = 3 + CONSUME_AFTER_RESCHEDULE = 5.minutes + def initialize(parent_table:, loose_foreign_key_definitions:, deleted_parent_records:, modification_tracker: LooseForeignKeys::ModificationTracker.new) @parent_table = parent_table @loose_foreign_key_definitions = loose_foreign_key_definitions @@ -11,15 +14,31 @@ module LooseForeignKeys :loose_foreign_key_processed_deleted_records, 'The number of processed loose foreign key deleted records' ) + @deleted_records_rescheduled_count = Gitlab::Metrics.counter( + :loose_foreign_key_rescheduled_deleted_records, + 'The number of rescheduled loose foreign key deleted records' + ) + @deleted_records_incremented_count = Gitlab::Metrics.counter( + :loose_foreign_key_incremented_deleted_records, + 'The number of loose foreign key deleted records with incremented cleanup_attempts' + ) end def execute loose_foreign_key_definitions.each do |loose_foreign_key_definition| run_cleaner_service(loose_foreign_key_definition, with_skip_locked: true) - break if modification_tracker.over_limit? + + if modification_tracker.over_limit? + handle_over_limit + break + end run_cleaner_service(loose_foreign_key_definition, with_skip_locked: false) - break if modification_tracker.over_limit? + + if modification_tracker.over_limit? + handle_over_limit + break + end end return if modification_tracker.over_limit? @@ -27,12 +46,33 @@ module LooseForeignKeys # At this point, all associations are cleaned up, we can update the status of the parent records update_count = LooseForeignKeys::DeletedRecord.mark_records_processed(deleted_parent_records) - deleted_records_counter.increment({ table: parent_table, db_config_name: LooseForeignKeys::DeletedRecord.connection.pool.db_config.name }, update_count) + deleted_records_counter.increment({ table: parent_table, db_config_name: db_config_name }, update_count) end private - attr_reader :parent_table, :loose_foreign_key_definitions, :deleted_parent_records, :modification_tracker, :deleted_records_counter + attr_reader :parent_table, :loose_foreign_key_definitions, :deleted_parent_records, :modification_tracker, :deleted_records_counter, :deleted_records_rescheduled_count, :deleted_records_incremented_count + + def handle_over_limit + return if Feature.disabled?(:lfk_fair_queueing) + + records_to_reschedule = [] + records_to_increment = [] + + deleted_parent_records.each do |deleted_record| + if deleted_record.cleanup_attempts >= CLEANUP_ATTEMPTS_BEFORE_RESCHEDULE + records_to_reschedule << deleted_record + else + records_to_increment << deleted_record + end + end + + reschedule_count = LooseForeignKeys::DeletedRecord.reschedule(records_to_reschedule, CONSUME_AFTER_RESCHEDULE.from_now) + deleted_records_rescheduled_count.increment({ table: parent_table, db_config_name: db_config_name }, reschedule_count) + + increment_count = LooseForeignKeys::DeletedRecord.increment_attempts(records_to_increment) + deleted_records_incremented_count.increment({ table: parent_table, db_config_name: db_config_name }, increment_count) + end def record_result(cleaner, result) if cleaner.async_delete? @@ -60,5 +100,9 @@ module LooseForeignKeys end end end + + def db_config_name + LooseForeignKeys::DeletedRecord.connection.pool.db_config.name + end end end diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index acd00d0d1ec..dc29bb2c6da 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -24,6 +24,9 @@ module Members add_members enqueue_onboarding_progress_action + + publish_event! + result rescue BlankInvitesError, TooManyInvitesError, MembershipLockedError => e error(e.message) @@ -144,6 +147,15 @@ module Members def formatted_errors errors.to_sentence end + + def publish_event! + Gitlab::EventStore.publish( + Members::MembersAddedEvent.new(data: { + source_id: source.id, + source_type: source.class.name + }) + ) + end end end diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb index e766a7e9044..fcce32ead94 100644 --- a/app/services/members/creator_service.rb +++ b/app/services/members/creator_service.rb @@ -67,6 +67,7 @@ module Members def create_member_task return unless member.persisted? return if member_task_attributes.value?(nil) + return if member.member_task.present? member.create_member_task(member_task_attributes) end diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index d2c83f82ff8..93a0d375b97 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -5,9 +5,8 @@ module MergeRequests include Gitlab::Utils::StrongMemoize def execute(merge_request) - prepare_for_mergeability(merge_request) if early_prepare_for_mergeability?(merge_request) + prepare_for_mergeability(merge_request) prepare_merge_request(merge_request) - mark_as_unchecked(merge_request) unless early_prepare_for_mergeability?(merge_request) end private @@ -15,7 +14,7 @@ module MergeRequests def prepare_for_mergeability(merge_request) create_pipeline_for(merge_request, current_user) merge_request.update_head_pipeline - mark_as_unchecked(merge_request) + check_mergeability(merge_request) end def prepare_merge_request(merge_request) @@ -26,11 +25,6 @@ module MergeRequests notification_service.new_merge_request(merge_request, current_user) - unless early_prepare_for_mergeability?(merge_request) - create_pipeline_for(merge_request, current_user) - merge_request.update_head_pipeline - end - merge_request.diffs(include_stats: false).write_cache merge_request.create_cross_references!(current_user) @@ -49,14 +43,13 @@ module MergeRequests LinkLfsObjectsService.new(project: merge_request.target_project).execute(merge_request) end - def early_prepare_for_mergeability?(merge_request) - strong_memoize("early_prepare_for_mergeability_#{merge_request.target_project_id}".to_sym) do - Feature.enabled?(:early_prepare_for_mergeability, merge_request.target_project) - end - end + def check_mergeability(merge_request) + return unless merge_request.preparing? - def mark_as_unchecked(merge_request) - merge_request.mark_as_unchecked if merge_request.preparing? + # Need to set to unchecked to be able to check for mergeability or else + # it'll be a no-op. + merge_request.mark_as_unchecked + merge_request.check_mergeability(async: true) end end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index b0d0c32abd1..3363fc90997 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -246,7 +246,9 @@ module MergeRequests def remove_all_attention_requests(merge_request) return unless merge_request.attention_requested_enabled? - ::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request).execute + users = merge_request.reviewers + merge_request.assignees + + ::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, users: users.uniq).execute end def remove_attention_requested(merge_request, user) diff --git a/app/services/merge_requests/bulk_remove_attention_requested_service.rb b/app/services/merge_requests/bulk_remove_attention_requested_service.rb index dd2ff741ba6..6573b623779 100644 --- a/app/services/merge_requests/bulk_remove_attention_requested_service.rb +++ b/app/services/merge_requests/bulk_remove_attention_requested_service.rb @@ -3,20 +3,24 @@ module MergeRequests class BulkRemoveAttentionRequestedService < MergeRequests::BaseService attr_accessor :merge_request + attr_accessor :users - def initialize(project:, current_user:, merge_request:) + def initialize(project:, current_user:, merge_request:, users:) super(project: project, current_user: current_user) @merge_request = merge_request + @users = users end + # rubocop: disable CodeReuse/ActiveRecord def execute return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) - merge_request.merge_request_assignees.update_all(state: :reviewed) - merge_request.merge_request_reviewers.update_all(state: :reviewed) + merge_request.merge_request_assignees.where(user_id: users).update_all(state: :reviewed) + merge_request.merge_request_reviewers.where(user_id: users).update_all(state: :reviewed) success end + # rubocop: enable CodeReuse/ActiveRecord end end diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb index 3e294aeaa07..30531fcc17b 100644 --- a/app/services/merge_requests/mergeability_check_service.rb +++ b/app/services/merge_requests/mergeability_check_service.rb @@ -14,8 +14,8 @@ module MergeRequests def async_execute return service_error if service_error - return unless merge_request.mark_as_checking + merge_request.mark_as_checking MergeRequestMergeabilityCheckWorker.perform_async(merge_request.id) end diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb index d1f45b4b49c..1c4e1784b34 100644 --- a/app/services/merge_requests/rebase_service.rb +++ b/app/services/merge_requests/rebase_service.rb @@ -2,7 +2,7 @@ module MergeRequests class RebaseService < MergeRequests::BaseService - REBASE_ERROR = 'Rebase failed. Please rebase locally' + REBASE_ERROR = 'Rebase failed: Rebase locally, resolve all conflicts, then push the branch.' attr_reader :merge_request, :rebase_error @@ -35,7 +35,7 @@ module MergeRequests def set_rebase_error(exception) @rebase_error = if exception.is_a?(Gitlab::Git::PreReceiveError) - "Something went wrong during the rebase pre-receive hook: #{exception.message}." + "The rebase pre-receive hook failed: #{exception.message}." else REBASE_ERROR end diff --git a/app/services/merge_requests/squash_service.rb b/app/services/merge_requests/squash_service.rb index 69b9740c2a5..f04682bf08a 100644 --- a/app/services/merge_requests/squash_service.rb +++ b/app/services/merge_requests/squash_service.rb @@ -9,9 +9,9 @@ module MergeRequests return success(squash_sha: merge_request.diff_head_sha) end - return error(s_('MergeRequests|This project does not allow squashing commits when merge requests are accepted.')) if squash_forbidden? + return error(s_("MergeRequests|Squashing not allowed: This project doesn't allow you to squash commits when merging.")) if squash_forbidden? - squash! || error(s_('MergeRequests|Failed to squash. Should be done manually.')) + squash! || error(s_('MergeRequests|Squashing failed: Squash the commits locally, resolve any conflicts, then push the branch.')) end private diff --git a/app/services/packages/maven/metadata/sync_service.rb b/app/services/packages/maven/metadata/sync_service.rb index 4f35db36fb0..dacf6750412 100644 --- a/app/services/packages/maven/metadata/sync_service.rb +++ b/app/services/packages/maven/metadata/sync_service.rb @@ -93,11 +93,7 @@ module Packages def metadata_package_file_for(package) return unless package - package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) - package.installable_package_files - else - package.package_files - end + package_files = package.installable_package_files package_files.with_file_name(Metadata.filename) .recent diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 55f16aa3e3d..e6b1b33a82a 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -33,6 +33,11 @@ module Projects SnippetsFinder.new(current_user, project: project).execute.select([:id, :title]) end + def contacts + Crm::ContactsFinder.new(current_user, group: project.group).execute + .select([:id, :email, :first_name, :last_name]) + end + def labels_as_hash(target) super(target, project_id: project.id, include_ancestor_groups: true) end diff --git a/app/services/projects/container_repository/delete_tags_service.rb b/app/services/projects/container_repository/delete_tags_service.rb index 410cf6c624e..b4a57c70111 100644 --- a/app/services/projects/container_repository/delete_tags_service.rb +++ b/app/services/projects/container_repository/delete_tags_service.rb @@ -14,6 +14,7 @@ module Projects @tag_names = params[:tags] return error('not tags specified') if @tag_names.blank? + return error('repository importing') if @container_repository.migration_importing? delete_tags end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 1d187b140ef..c885369dfec 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -89,7 +89,7 @@ module Projects end def after_create_actions - log_info("#{@project.owner.name} created a new project \"#{@project.full_name}\"") + log_info("#{current_user.name} created a new project \"#{@project.full_name}\"") if @project.import? experiment(:combined_registration, user: current_user).track(:import_project) @@ -167,7 +167,7 @@ module Projects end def readme_content - @readme_template.presence || experiment(:new_project_readme_content, namespace: @project.namespace).run_with(@project) + @readme_template.presence || ReadmeRendererService.new(@project, current_user).execute end def skip_wiki? diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 5c4a0e947de..95af5a6863f 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -37,6 +37,8 @@ module Projects system_hook_service.execute_hooks_for(project, :destroy) log_info("Project \"#{project.full_path}\" was deleted") + publish_project_deleted_event_for(project) if Feature.enabled?(:publish_project_deleted_event, default_enabled: :yaml) + current_user.invalidate_personal_projects_count true @@ -139,6 +141,7 @@ module Projects destroy_web_hooks! destroy_project_bots! destroy_ci_records! + destroy_mr_diff_commits! # Rails attempts to load all related records into memory before # destroying: https://github.com/rails/rails/issues/22510 @@ -154,6 +157,33 @@ module Projects log_info("Attempting to destroy #{project.full_path} (#{project.id})") end + # Projects will have at least one merge_request_diff_commit for every commit + # contained in every MR, which deleting via `project.destroy!` and + # cascading deletes may exceed statement timeouts, causing failures. + # (see https://gitlab.com/gitlab-org/gitlab/-/issues/346166) + # + # rubocop: disable CodeReuse/ActiveRecord + def destroy_mr_diff_commits! + mr_batch_size = 100 + delete_batch_size = 1000 + + project.merge_requests.each_batch(column: :iid, of: mr_batch_size) do |relation_ids| + loop do + inner_query = MergeRequestDiffCommit + .select(:merge_request_diff_id, :relative_order) + .where(merge_request_diff_id: MergeRequestDiff.where(merge_request_id: relation_ids).select(:id)) + .limit(delete_batch_size) + + deleted_rows = MergeRequestDiffCommit + .where('(merge_request_diff_commits.merge_request_diff_id, merge_request_diff_commits.relative_order) IN (?)', inner_query) + .delete_all + + break if deleted_rows == 0 + end + end + end + # rubocop: enable CodeReuse/ActiveRecord + def destroy_ci_records! project.all_pipelines.find_each(batch_size: BATCH_SIZE) do |pipeline| # rubocop: disable CodeReuse/ActiveRecord # Destroy artifacts, then builds, then pipelines @@ -232,6 +262,12 @@ module Projects def flush_caches(project) Projects::ForksCountService.new(project).delete_cache end + + def publish_project_deleted_event_for(project) + data = { project_id: project.id, namespace_id: project.namespace_id } + event = Projects::ProjectDeletedEvent.new(data: data) + Gitlab::EventStore.publish(event) + end end end diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index b1a2182fbdc..b91b7f34d42 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -47,8 +47,7 @@ module Projects end def save_all! - if save_exporters - Gitlab::ImportExport::Saver.save(exportable: project, shared: shared) + if save_exporters && save_export_archive notify_success else notify_error! @@ -59,6 +58,10 @@ module Projects exporters.all?(&:save) end + def save_export_archive + Gitlab::ImportExport::Saver.save(exportable: project, shared: shared) + end + def exporters [ version_saver, avatar_saver, project_tree_saver, uploads_saver, diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index fe9dce26029..9da72d9300e 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -34,7 +34,7 @@ module Projects def wrap_download_errors(&block) yield rescue SizeError, OidError, ResponseError, StandardError => e - error("LFS file with oid #{lfs_oid} could't be downloaded from #{lfs_sanitized_url}: #{e.message}") + error("LFS file with oid #{lfs_oid} couldn't be downloaded from #{lfs_sanitized_url}: #{e.message}") end def download_lfs_file! @@ -104,7 +104,7 @@ module Projects rescue StandardError => e # If the lfs file is successfully downloaded it will be removed # when it is added to the project's lfs files. - # Nevertheless if any excetion raises the file would remain + # Nevertheless if any exception raises the file would remain # in the file system. Here we ensure to remove it File.unlink(file) if File.exist?(file) diff --git a/app/services/projects/overwrite_project_service.rb b/app/services/projects/overwrite_project_service.rb index c58fba33b2a..eea8f867b45 100644 --- a/app/services/projects/overwrite_project_service.rb +++ b/app/services/projects/overwrite_project_service.rb @@ -6,30 +6,34 @@ module Projects return unless source_project && source_project.namespace_id == @project.namespace_id start_time = ::Gitlab::Metrics::System.monotonic_time + original_source_name = source_project.name + original_source_path = source_project.path + tmp_source_name, tmp_source_path = tmp_source_project_name(source_project) - Project.transaction do - move_before_destroy_relationships(source_project) - # Reset is required in order to get the proper - # uncached fork network method calls value. - ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/340256') do - destroy_old_project(source_project.reset) - end - rename_project(source_project.name, source_project.path) - - @project + move_relationships_between(source_project, @project) + + source_project_rename = rename_project(source_project, tmp_source_name, tmp_source_path) + + if source_project_rename[:status] == :error + raise 'Source project rename failed during project overwrite' end - # Projects::DestroyService can raise Exceptions, but we don't want - # to pass that kind of exception to the caller. Instead, we change it - # for a StandardError exception - rescue Exception => e # rubocop:disable Lint/RescueException - attempt_restore_repositories(source_project) - - if e.instance_of?(Exception) - raise StandardError, e.message - else - raise + + new_project_rename = rename_project(@project, original_source_name, original_source_path) + + if new_project_rename[:status] == :error + rename_project(source_project, original_source_name, original_source_path) + + raise 'New project rename failed during project overwrite' end + schedule_source_project_deletion(source_project) + + @project + rescue StandardError => e + move_relationships_between(@project, source_project) + remove_source_project_from_fork_network(source_project) + + raise e ensure track_service(start_time, source_project, e) end @@ -48,45 +52,63 @@ module Projects error: exception.class.name) end - def move_before_destroy_relationships(source_project) + def move_relationships_between(source_project, target_project) options = { remove_remaining_elements: false } - ::Projects::MoveUsersStarProjectsService.new(@project, @current_user).execute(source_project, **options) - ::Projects::MoveAccessService.new(@project, @current_user).execute(source_project, **options) - ::Projects::MoveDeployKeysProjectsService.new(@project, @current_user).execute(source_project, **options) - ::Projects::MoveNotificationSettingsService.new(@project, @current_user).execute(source_project, **options) - ::Projects::MoveForksService.new(@project, @current_user).execute(source_project, **options) - ::Projects::MoveLfsObjectsProjectsService.new(@project, @current_user).execute(source_project, **options) - add_source_project_to_fork_network(source_project) - end - - def destroy_old_project(source_project) - # Delete previous project (synchronously) and unlink relations - ::Projects::DestroyService.new(source_project, @current_user).execute + Project.transaction do + ::Projects::MoveUsersStarProjectsService.new(target_project, @current_user).execute(source_project, **options) + ::Projects::MoveAccessService.new(target_project, @current_user).execute(source_project, **options) + ::Projects::MoveDeployKeysProjectsService.new(target_project, @current_user).execute(source_project, **options) + ::Projects::MoveNotificationSettingsService.new(target_project, @current_user).execute(source_project, **options) + ::Projects::MoveForksService.new(target_project, @current_user).execute(source_project, **options) + ::Projects::MoveLfsObjectsProjectsService.new(target_project, @current_user).execute(source_project, **options) + + add_source_project_to_fork_network(source_project) + end end - def rename_project(name, path) - # Update de project's name and path to the original name/path - ::Projects::UpdateService.new(@project, - @current_user, - { name: name, path: path }) - .execute + def schedule_source_project_deletion(source_project) + ::Projects::DestroyService.new(source_project, @current_user).async_execute end - def attempt_restore_repositories(project) - ::Projects::DestroyRollbackService.new(project, @current_user).execute + def rename_project(target_project, name, path) + ::Projects::UpdateService.new(target_project, @current_user, { name: name, path: path }).execute end def add_source_project_to_fork_network(source_project) - return unless @project.fork_network + return if source_project == @project + return unless fork_network # Because they have moved all references in the fork network from the source_project # we won't be able to query the database (only through its cached data), # for its former relationships. That's why we're adding it to the network # as a fork of the target project - ForkNetworkMember.create!(fork_network: @project.fork_network, + ForkNetworkMember.create!(fork_network: fork_network, project: source_project, forked_from_project: @project) end + + def remove_source_project_from_fork_network(source_project) + return unless fork_network + + fork_member = ForkNetworkMember.find_by( # rubocop: disable CodeReuse/ActiveRecord + fork_network: fork_network, + project: source_project, + forked_from_project: @project) + + fork_member&.destroy + end + + def tmp_source_project_name(source_project) + random_string = SecureRandom.hex + tmp_name = "#{source_project.name}-old-#{random_string}" + tmp_path = "#{source_project.path}-old-#{random_string}" + + [tmp_name, tmp_path] + end + + def fork_network + @project.fork_network_member&.fork_network + end end end diff --git a/app/services/projects/readme_renderer_service.rb b/app/services/projects/readme_renderer_service.rb new file mode 100644 index 00000000000..6871976aded --- /dev/null +++ b/app/services/projects/readme_renderer_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Projects + class ReadmeRendererService < BaseService + include Rails.application.routes.url_helpers + + TEMPLATE_PATH = Rails.root.join('app', 'views', 'projects', 'readme_templates') + + def execute + render(params[:template_name] || :default) + end + + private + + def render(template_name) + ERB.new(File.read(sanitized_filename(template_name)), trim_mode: '<>').result(binding) + end + + def sanitized_filename(template_name) + path = Gitlab::Utils.check_path_traversal!("#{template_name}.md.tt") + path = TEMPLATE_PATH.join(path).to_s + Gitlab::Utils.check_allowed_absolute_path!(path, [TEMPLATE_PATH.to_s]) + + path + end + end +end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 17da77fe950..51c0989ee55 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -2,11 +2,11 @@ # Projects::TransferService class # -# Used for transfer project to another namespace +# Used to transfer a project to another namespace # # Ex. -# # Move projects to namespace with ID 17 by user -# Projects::TransferService.new(project, user, namespace_id: 17).execute +# # Move project to namespace by user +# Projects::TransferService.new(project, user).execute(namespace) # module Projects class TransferService < BaseService @@ -103,6 +103,8 @@ module Projects update_repository_configuration(@new_path) + remove_issue_contacts + execute_system_hooks end @@ -254,6 +256,12 @@ module Projects namespace_traversal_ids: new_namespace.traversal_ids } end + + def remove_issue_contacts + return unless @old_group&.root_ancestor != @new_namespace&.root_ancestor + + CustomerRelations::IssueContact.delete_for_project(project.id) + end end end diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index ab489ba49ca..906c4b98f56 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -61,28 +61,32 @@ module QuickActions private + def failed_parse(message) + raise Gitlab::QuickActions::CommandDefinition::ParseError, message + end + def extractor Gitlab::QuickActions::Extractor.new(self.class.command_definitions) end - # rubocop: disable CodeReuse/ActiveRecord def extract_users(params) - return [] if params.nil? + return [] if params.blank? - users = extract_references(params, :user) + # We are using the a simple User.by_username query here rather than a ReferenceExtractor + # because the needs here are much simpler: we only deal in usernames, and + # want to also handle bare usernames. The ReferenceExtractor also has + # different behaviour, and will return all group members for groups named + # using a user-style reference, which is not in scope here. + args = params.split(/\s|,/).select(&:present?).uniq - ['and'] + usernames = (args - ['me']).map { _1.delete_prefix('@') } + found = User.by_username(usernames).to_a.select { can?(:read_user_profile, _1) } + found_names = found.map(&:username).to_set + missing = args.reject { |arg| arg == 'me' || found_names.include?(arg.delete_prefix('@')) }.map { "'#{_1}'" } - if users.empty? - users = - if params.strip == 'me' - [current_user] - else - User.where(username: params.split(' ').map(&:strip)) - end - end + failed_parse(format(_("Failed to find users for %{missing}"), missing: missing.to_sentence)) if missing.present? - users + found + [current_user].select { args.include?('me') } end - # rubocop: enable CodeReuse/ActiveRecord def find_milestones(project, params = {}) group_ids = project.group.self_and_ancestors.select(:id) if project.group @@ -187,6 +191,10 @@ module QuickActions user: current_user ) end + + def can?(ability, object) + Ability.allowed?(current_user, ability, object) + end end end diff --git a/app/services/resource_events/change_state_service.rb b/app/services/resource_events/change_state_service.rb index d68b86a1513..a396f7a1907 100644 --- a/app/services/resource_events/change_state_service.rb +++ b/app/services/resource_events/change_state_service.rb @@ -14,7 +14,7 @@ module ResourceEvents ResourceStateEvent.create( user: user, - resource.class.underscore => resource, + resource.noteable_target_type_name => resource, source_commit: commit_id_of(mentionable_source), source_merge_request_id: merge_request_id_of(mentionable_source), state: ResourceStateEvent.states[state], diff --git a/app/services/security/ci_configuration/container_scanning_create_service.rb b/app/services/security/ci_configuration/container_scanning_create_service.rb new file mode 100644 index 00000000000..788533575e6 --- /dev/null +++ b/app/services/security/ci_configuration/container_scanning_create_service.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Security + module CiConfiguration + class ContainerScanningCreateService < ::Security::CiConfiguration::BaseCreateService + private + + def action + Security::CiConfiguration::ContainerScanningBuildAction.new(project.auto_devops_enabled?, existing_gitlab_ci_content).generate + end + + def next_branch + 'set-container-scanning-config' + end + + def message + _('Configure Container Scanning in `.gitlab-ci.yml`, creating this file if it does not already exist') + end + + def description + _('Configure Container Scanning in `.gitlab-ci.yml` using the GitLab managed template. You can [add variable overrides](https://docs.gitlab.com/ee/user/application_security/container_scanning/#customizing-the-container-scanning-settings) to customize Container Scanning settings.') + end + end + end +end diff --git a/app/services/service_ping/build_payload_service.rb b/app/services/service_ping/build_payload_service.rb index 2bef3d32103..f4ae939fd07 100644 --- a/app/services/service_ping/build_payload_service.rb +++ b/app/services/service_ping/build_payload_service.rb @@ -19,7 +19,7 @@ module ServicePing end def raw_payload - @raw_payload ||= ::Gitlab::UsageData.data(force_refresh: true) + @raw_payload ||= ::Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values) end end end diff --git a/app/services/service_ping/submit_service.rb b/app/services/service_ping/submit_service.rb index d3d9dcecb2b..c8733bc2f11 100644 --- a/app/services/service_ping/submit_service.rb +++ b/app/services/service_ping/submit_service.rb @@ -5,6 +5,7 @@ module ServicePing PRODUCTION_BASE_URL = 'https://version.gitlab.com' STAGING_BASE_URL = 'https://gitlab-services-version-gitlab-com-staging.gs-staging.gitlab.org' USAGE_DATA_PATH = 'usage_data' + ERROR_PATH = 'usage_ping_errors' SubmissionError = Class.new(StandardError) @@ -15,13 +16,24 @@ module ServicePing def execute return unless ServicePing::ServicePingSettings.product_intelligence_enabled? + start = Time.current begin usage_data = BuildPayloadService.new.execute response = submit_usage_data_payload(usage_data) - rescue StandardError + rescue StandardError => e return unless Gitlab::CurrentSettings.usage_ping_enabled? - usage_data = Gitlab::UsageData.data(force_refresh: true) + error_payload = { + time: Time.current, + uuid: Gitlab::UsageData.add_metric('UuidMetric'), + hostname: Gitlab::UsageData.add_metric('HostnameMetric'), + version: Gitlab::UsageData.alt_usage_data { Gitlab::VERSION }, + message: e.message, + elapsed: (Time.current - start).round(1) + } + submit_payload({ error: error_payload }, url: error_url) + + usage_data = Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values) response = submit_usage_data_payload(usage_data) end @@ -42,12 +54,16 @@ module ServicePing URI.join(base_url, USAGE_DATA_PATH) end + def error_url + URI.join(base_url, ERROR_PATH) + end + private - def submit_payload(usage_data) + def submit_payload(payload, url: self.url) Gitlab::HTTP.post( url, - body: usage_data.to_json, + body: payload.to_json, allow_local_requests: true, headers: { 'Content-type' => 'application/json' } ) diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 0d13c73d49d..1f1edad7a69 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -45,6 +45,10 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: issuable, project: project, author: author).change_issuable_reviewers(old_reviewers) end + def change_issuable_contacts(issuable, project, author, added_count, removed_count) + ::SystemNotes::IssuablesService.new(noteable: issuable, project: project, author: author).change_issuable_contacts(added_count, removed_count) + end + def relate_issue(noteable, noteable_ref, user) ::SystemNotes::IssuablesService.new(noteable: noteable, project: noteable.project, author: user).relate_issue(noteable_ref) end @@ -319,8 +323,8 @@ module SystemNoteService merge_requests_service(noteable, noteable.project, user).unapprove_mr end - def change_alert_status(alert, author) - ::SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: author).change_alert_status(alert) + def change_alert_status(alert, author, reason = nil) + ::SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: author).change_alert_status(reason) end def new_alert_issue(alert, issue, author) @@ -335,8 +339,8 @@ module SystemNoteService ::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).change_incident_severity end - def resolve_incident_status(incident, author) - ::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).resolve_incident_status + def change_incident_status(incident, author, reason = nil) + ::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).change_incident_status(reason) end def log_resolving_alert(alert, monitoring_tool) diff --git a/app/services/system_notes/alert_management_service.rb b/app/services/system_notes/alert_management_service.rb index 70cdd5c6434..994e3174668 100644 --- a/app/services/system_notes/alert_management_service.rb +++ b/app/services/system_notes/alert_management_service.rb @@ -24,11 +24,12 @@ module SystemNotes # Example Note text: # # "changed the status to Acknowledged" + # "changed the status to Acknowledged by changing the incident status of #540" # # Returns the created Note object - def change_alert_status(alert) - status = alert.state.to_s.titleize - body = "changed the status to **#{status}**" + def change_alert_status(reason) + status = noteable.state.to_s.titleize + body = "changed the status to **#{status}**#{reason}" create_note(NoteSummary.new(noteable, project, author, body, action: 'status')) end @@ -39,30 +40,15 @@ module SystemNotes # # Example Note text: # - # "created issue #17 for this alert" + # "created incident #17 for this alert" # # Returns the created Note object def new_alert_issue(issue) - body = "created issue #{issue.to_reference(project)} for this alert" + body = "created incident #{issue.to_reference(project)} for this alert" create_note(NoteSummary.new(noteable, project, author, body, action: 'alert_issue_added')) end - # Called when an AlertManagement::Alert is resolved due to the associated issue being closed - # - # issue - Issue object. - # - # Example Note text: - # - # "changed the status to Resolved by closing issue #17" - # - # Returns the created Note object - def closed_alert_issue(issue) - body = "changed the status to **Resolved** by closing issue #{issue.to_reference(project)}" - - create_note(NoteSummary.new(noteable, project, author, body, action: 'status')) - end - # Called when an alert is resolved due to received resolving alert payload # # alert - AlertManagement::Alert object. diff --git a/app/services/system_notes/incident_service.rb b/app/services/system_notes/incident_service.rb index 785291e0637..f3f9dfbec96 100644 --- a/app/services/system_notes/incident_service.rb +++ b/app/services/system_notes/incident_service.rb @@ -26,8 +26,19 @@ module SystemNotes end end - def resolve_incident_status - body = 'changed the status to **Resolved** by closing the incident' + # Called when the status of an IncidentManagement::IssuableEscalationStatus has changed + # + # reason - String. + # + # Example Note text: + # + # "changed the incident status to Acknowledged" + # "changed the incident status to Acknowledged by changing the status of ^alert#540" + # + # Returns the created Note object + def change_incident_status(reason) + status = noteable.escalation_status.status_name.to_s.titleize + body = "changed the incident status to **#{status}**#{reason}" create_note(NoteSummary.new(noteable, project, author, body, action: 'status')) end diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index d33dcd65589..09f36bb6501 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -111,6 +111,35 @@ module SystemNotes create_note(NoteSummary.new(noteable, project, author, body, action: 'reviewer')) end + # Called when the contacts of an issuable are changed or removed + # We intend to reference the contacts but for security we are just + # going to state how many were added/removed for now. See discussion: + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77816#note_806114273 + # + # added_count - number of contacts added, or 0 + # removed_count - number of contacts removed, or 0 + # + # Example Note text: + # + # "added 2 contacts" + # + # "added 3 contacts and removed one contact" + # + # Returns the created Note object + def change_issuable_contacts(added_count, removed_count) + text_parts = [] + + Gitlab::I18n.with_default_locale do + text_parts << "added #{added_count} #{'contact'.pluralize(added_count)}" if added_count > 0 + text_parts << "removed #{removed_count} #{'contact'.pluralize(removed_count)}" if removed_count > 0 + end + + return if text_parts.empty? + + body = text_parts.join(' and ') + create_note(NoteSummary.new(noteable, project, author, body, action: 'contact')) + end + # Called when the title of a Noteable is changed # # old_title - Previous String title diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb index 32cfa198ce8..082fa1447fc 100644 --- a/app/services/task_list_toggle_service.rb +++ b/app/services/task_list_toggle_service.rb @@ -38,7 +38,7 @@ class TaskListToggleService return unless markdown_task.chomp == line_source return unless source_checkbox = Taskable::ITEM_PATTERN.match(markdown_task) - currently_checked = TaskList::Item.new(source_checkbox[1]).complete? + currently_checked = TaskList::Item.new(source_checkbox[2]).complete? # Check `toggle_as_checked` to make sure we don't accidentally replace # any `[ ]` or `[x]` in the middle of the text diff --git a/app/services/test_hooks/base_service.rb b/app/services/test_hooks/base_service.rb index 0fda6fb1ed0..b41a9959c13 100644 --- a/app/services/test_hooks/base_service.rb +++ b/app/services/test_hooks/base_service.rb @@ -18,7 +18,7 @@ module TestHooks return error('Testing not available for this hook') if trigger_key.nil? || data.blank? return error(data[:error]) if data[:error].present? - hook.execute(data, trigger_key) + hook.execute(data, trigger_key, force: true) end end end diff --git a/app/services/update_container_registry_info_service.rb b/app/services/update_container_registry_info_service.rb index 531335839a9..7d79b257687 100644 --- a/app/services/update_container_registry_info_service.rb +++ b/app/services/update_container_registry_info_service.rb @@ -15,6 +15,12 @@ class UpdateContainerRegistryInfoService client = ContainerRegistry::Client.new(registry_config.api_url, token: token) info = client.registry_info + gitlab_api_client = ContainerRegistry::GitlabApiClient.new(registry_config.api_url, token: token) + if gitlab_api_client.supports_gitlab_api? + info[:features] ||= [] + info[:features] << ContainerRegistry::GitlabApiClient::REGISTRY_GITLAB_V1_API_FEATURE + end + Gitlab::CurrentSettings.update!( container_registry_vendor: info[:vendor] || '', container_registry_version: info[:version] || '', diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 1634cc017ae..4ec875098fa 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -65,10 +65,7 @@ module Users user.destroy_dependent_associations_in_batches(exclude: [:snippets]) # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing - user_data = nil - ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/340260') do - user_data = user.destroy - end + user_data = user.destroy namespace.destroy user_data diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 33e34ec41e2..b1d8872aa5e 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -34,11 +34,12 @@ class WebHookService hook_name.to_s.singularize.titleize end - def initialize(hook, data, hook_name, uniqueness_token = nil) + def initialize(hook, data, hook_name, uniqueness_token = nil, force: false) @hook = hook @data = data @hook_name = hook_name.to_s @uniqueness_token = uniqueness_token + @force = force @request_options = { timeout: Gitlab.config.gitlab.webhook_timeout, use_read_total_timeout: true, @@ -46,10 +47,17 @@ class WebHookService } end + def disabled? + !@force && !hook.executable? + end + def execute - return { status: :error, message: 'Hook disabled' } unless hook.executable? + return { status: :error, message: 'Hook disabled' } if disabled? - log_recursion_limit if recursion_blocked? + if recursion_blocked? + log_recursion_blocked + return { status: :error, message: 'Recursive webhook blocked' } + end Gitlab::WebHooks::RecursionDetection.register!(hook) @@ -96,13 +104,14 @@ class WebHookService def async_execute Gitlab::ApplicationContext.with_context(hook.application_context) do - break log_rate_limit if rate_limited? + break log_rate_limited if rate_limited? + break log_recursion_blocked if recursion_blocked? - log_recursion_limit if recursion_blocked? + params = { + recursion_detection_request_uuid: Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid + }.compact - data[:_gitlab_recursion_detection_request_uuid] = Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid - - WebHookWorker.perform_async(hook.id, data, hook_name) + WebHookWorker.perform_async(hook.id, data, hook_name, params) end end @@ -144,8 +153,12 @@ class WebHookService internal_error_message: error_message } - ::WebHooks::LogExecutionWorker - .perform_async(hook.id, log_data, category, uniqueness_token) + if @force # executed as part of test - run log-execution inline. + ::WebHooks::LogExecutionService.new(hook: hook, log_data: log_data, response_category: category).execute + else + ::WebHooks::LogExecutionWorker + .perform_async(hook.id, log_data, category, uniqueness_token) + end end def response_category(response) @@ -202,7 +215,7 @@ class WebHookService @rate_limit ||= hook.rate_limit end - def log_rate_limit + def log_rate_limited Gitlab::AuthLogger.error( message: 'Webhook rate limit exceeded', hook_id: hook.id, @@ -212,9 +225,9 @@ class WebHookService ) end - def log_recursion_limit + def log_recursion_blocked Gitlab::AuthLogger.error( - message: 'Webhook recursion detected and will be blocked in future', + message: 'Recursive webhook blocked from executing', hook_id: hook.id, hook_type: hook.type, hook_name: hook_name, diff --git a/app/services/work_items/create_service.rb b/app/services/work_items/create_service.rb index 49f7b89158b..705735fe403 100644 --- a/app/services/work_items/create_service.rb +++ b/app/services/work_items/create_service.rb @@ -2,6 +2,8 @@ module WorkItems class CreateService + include ::Services::ReturnServiceResponses + def initialize(project:, current_user: nil, params: {}, spam_params:) @create_service = ::Issues::CreateService.new( project: project, @@ -10,10 +12,28 @@ module WorkItems spam_params: spam_params, build_service: ::WorkItems::BuildService.new(project: project, current_user: current_user, params: params) ) + @current_user = current_user + @project = project end def execute - @create_service.execute + unless @current_user.can?(:create_work_item, @project) + return error(_('Operation not allowed'), :forbidden) + end + + work_item = @create_service.execute + + if work_item.valid? + success(payload(work_item)) + else + error(work_item.errors.full_messages, :unprocessable_entity, pass_back: payload(work_item)) + end + end + + private + + def payload(work_item) + { work_item: work_item } end end end diff --git a/app/services/work_items/delete_service.rb b/app/services/work_items/delete_service.rb new file mode 100644 index 00000000000..1093a403a1c --- /dev/null +++ b/app/services/work_items/delete_service.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module WorkItems + class DeleteService < Issuable::DestroyService + def execute(work_item) + unless current_user.can?(:delete_work_item, work_item) + return ::ServiceResponse.error(message: 'User not authorized to delete work item') + end + + if super + ::ServiceResponse.success + else + ::ServiceResponse.error(message: work_item.errors.full_messages) + end + end + end +end diff --git a/app/services/work_items/update_service.rb b/app/services/work_items/update_service.rb new file mode 100644 index 00000000000..5c45f4d90e5 --- /dev/null +++ b/app/services/work_items/update_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module WorkItems + class UpdateService < ::Issues::UpdateService + private + + def after_update(issuable) + super + + GraphqlTriggers.issuable_title_updated(issuable) if issuable.previous_changes.key?(:title) + end + end +end |