diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-10-20 09:40:42 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-10-20 09:40:42 +0000 |
commit | ee664acb356f8123f4f6b00b73c1e1cf0866c7fb (patch) | |
tree | f8479f94a28f66654c6a4f6fb99bad6b4e86a40e /app/services | |
parent | 62f7d5c5b69180e82ae8196b7b429eeffc8e7b4f (diff) | |
download | gitlab-ce-ee664acb356f8123f4f6b00b73c1e1cf0866c7fb.tar.gz |
Add latest changes from gitlab-org/gitlab@15-5-stable-eev15.5.0-rc42
Diffstat (limited to 'app/services')
98 files changed, 1322 insertions, 442 deletions
diff --git a/app/services/admin/set_feature_flag_service.rb b/app/services/admin/set_feature_flag_service.rb new file mode 100644 index 00000000000..d72a18a6a58 --- /dev/null +++ b/app/services/admin/set_feature_flag_service.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module Admin + class SetFeatureFlagService + def initialize(feature_flag_name:, params:) + @name = feature_flag_name + @params = params + end + + def execute + unless params[:force] + error = validate_feature_flag_name + return ServiceResponse.error(message: error, reason: :invalid_feature_flag) if error + end + + flag_target = Feature::Target.new(params) + value = gate_value(params) + + case value + when true + enable!(flag_target) + when false + disable!(flag_target) + else + enable_partially!(value, params) + end + + feature_flag = Feature.get(name) # rubocop:disable Gitlab/AvoidFeatureGet + + ServiceResponse.success(payload: { feature_flag: feature_flag }) + rescue Feature::Target::UnknowTargetError => e + ServiceResponse.error(message: e.message, reason: :actor_not_found) + end + + private + + attr_reader :name, :params + + def enable!(flag_target) + if flag_target.gate_specified? + flag_target.targets.each { |target| Feature.enable(name, target) } + else + Feature.enable(name) + end + end + + def disable!(flag_target) + if flag_target.gate_specified? + flag_target.targets.each { |target| Feature.disable(name, target) } + else + Feature.disable(name) + end + end + + def enable_partially!(value, params) + if params[:key] == 'percentage_of_actors' + Feature.enable_percentage_of_actors(name, value) + else + Feature.enable_percentage_of_time(name, value) + end + end + + def validate_feature_flag_name + # overridden in EE + end + + def gate_value(params) + case params[:value] + when 'true' + true + when '0', 'false' + false + else + # https://github.com/jnunemaker/flipper/blob/master/lib/flipper/typecast.rb#L47 + if params[:value].to_s.include?('.') + params[:value].to_f + else + params[:value].to_i + end + end + end + end +end + +Admin::SetFeatureFlagService.prepend_mod diff --git a/app/services/alert_management/create_alert_issue_service.rb b/app/services/alert_management/create_alert_issue_service.rb index 34c2003bd01..28e312a6fa3 100644 --- a/app/services/alert_management/create_alert_issue_service.rb +++ b/app/services/alert_management/create_alert_issue_service.rb @@ -21,7 +21,7 @@ module AlertManagement result = create_incident return result unless result.success? - issue = result.payload[:issue] + issue = result[:issue] perform_after_create_tasks(issue) result diff --git a/app/services/authorized_project_update/project_recalculate_service.rb b/app/services/authorized_project_update/project_recalculate_service.rb index e0b8158417c..8d60fffd959 100644 --- a/app/services/authorized_project_update/project_recalculate_service.rb +++ b/app/services/authorized_project_update/project_recalculate_service.rb @@ -64,7 +64,12 @@ module AuthorizedProjectUpdate end def refresh_authorizations - project.remove_project_authorizations(user_ids_to_remove) if user_ids_to_remove.any? + if user_ids_to_remove.any? + ProjectAuthorization.delete_all_in_batches_for_project( + project: project, + user_ids: user_ids_to_remove) + end + ProjectAuthorization.insert_all_in_batches(authorizations_to_create) if authorizations_to_create.any? end diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index 465025ef2e9..fcaa74555ca 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -50,7 +50,7 @@ module Boards end def set_issue_types - params[:issue_types] ||= Issue::TYPES_FOR_LIST + params[:issue_types] ||= Issue::TYPES_FOR_BOARD_LIST end def item_model diff --git a/app/services/boards/lists/generate_service.rb b/app/services/boards/lists/generate_service.rb deleted file mode 100644 index d74320e92a3..00000000000 --- a/app/services/boards/lists/generate_service.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -module Boards - module Lists - class GenerateService < Boards::BaseService - def execute(board) - return false unless board.lists.movable.empty? - - List.transaction do - label_params.each do |params| - response = create_list(board, params) - - raise ActiveRecord::Rollback unless response.success? - end - end - - true - end - - private - - def create_list(board, params) - label = find_or_create_label(params) - Lists::CreateService.new(parent, current_user, label_id: label.id).execute(board) - end - - def find_or_create_label(params) - ::Labels::FindOrCreateService.new(current_user, parent, params).execute - end - - def label_params - [ - { name: 'To Do', color: '#F0AD4E' }, - { name: 'Doing', color: '#5CB85C' } - ] - end - end - end -end diff --git a/app/services/boards/lists/list_service.rb b/app/services/boards/lists/list_service.rb index e81ef467a4e..cf15db4314c 100644 --- a/app/services/boards/lists/list_service.rb +++ b/app/services/boards/lists/list_service.rb @@ -9,23 +9,27 @@ module Boards end lists = board.lists.preload_associated_models + lists = lists.with_types(available_list_types_for(board)) return lists.id_in(params[:list_id]) if params[:list_id].present? - list_types = unavailable_list_types_for(board) - lists.without_types(list_types) + lists end private - def unavailable_list_types_for(board) - hidden_lists_for(board) + def available_list_types_for(board) + licensed_list_types(board) + visible_lists(board) end - def hidden_lists_for(board) - [].tap do |hidden| - hidden << ::List.list_types[:backlog] if board.hide_backlog_list? - hidden << ::List.list_types[:closed] if board.hide_closed_list? + def licensed_list_types(board) + [List.list_types[:label]] + end + + def visible_lists(board) + [].tap do |visible| + visible << ::List.list_types[:backlog] unless board.hide_backlog_list? + visible << ::List.list_types[:closed] unless board.hide_closed_list? end end end diff --git a/app/services/bulk_imports/create_pipeline_trackers_service.rb b/app/services/bulk_imports/create_pipeline_trackers_service.rb index af97aec09b5..f5b944e6df5 100644 --- a/app/services/bulk_imports/create_pipeline_trackers_service.rb +++ b/app/services/bulk_imports/create_pipeline_trackers_service.rb @@ -53,11 +53,13 @@ module BulkImports def log_skipped_pipeline(pipeline, minimum_version, maximum_version) logger.info( message: 'Pipeline skipped as source instance version not compatible with pipeline', - entity_id: entity.id, + bulk_import_entity_id: entity.id, + bulk_import_id: entity.bulk_import_id, pipeline_name: pipeline[:pipeline], minimum_source_version: minimum_version, maximum_source_version: maximum_version, - source_version: source_version.to_s + source_version: source_version.to_s, + importer: 'gitlab_migration' ) end diff --git a/app/services/bulk_imports/create_service.rb b/app/services/bulk_imports/create_service.rb index 31e1a822e78..d3c6dcca588 100644 --- a/app/services/bulk_imports/create_service.rb +++ b/app/services/bulk_imports/create_service.rb @@ -38,6 +38,8 @@ module BulkImports def execute bulk_import = create_bulk_import + Gitlab::Tracking.event(self.class.name, 'create', label: 'bulk_import_group') + BulkImportWorker.perform_async(bulk_import.id) ServiceResponse.success(payload: bulk_import) diff --git a/app/services/bulk_imports/repository_bundle_export_service.rb b/app/services/bulk_imports/repository_bundle_export_service.rb index 31a2ed6d1af..86159f5189d 100644 --- a/app/services/bulk_imports/repository_bundle_export_service.rb +++ b/app/services/bulk_imports/repository_bundle_export_service.rb @@ -9,13 +9,19 @@ module BulkImports end def execute - repository.bundle_to_disk(bundle_filepath) if repository.exists? + return unless repository_exists? + + repository.bundle_to_disk(bundle_filepath) end private attr_reader :repository, :export_path, :export_filename + def repository_exists? + repository.exists? && !repository.empty? + end + def bundle_filepath File.join(export_path, "#{export_filename}.bundle") end diff --git a/app/services/bulk_imports/uploads_export_service.rb b/app/services/bulk_imports/uploads_export_service.rb index 7f5ee7b8624..315590bea31 100644 --- a/app/services/bulk_imports/uploads_export_service.rb +++ b/app/services/bulk_imports/uploads_export_service.rb @@ -22,8 +22,9 @@ module BulkImports subdir_path = export_subdir_path(upload) mkdir_p(subdir_path) download_or_copy_upload(uploader, File.join(subdir_path, uploader.filename)) - rescue Errno::ENAMETOOLONG => e - # Do not fail entire export process if downloaded file has filename that exceeds 255 characters. + rescue StandardError => e + # Do not fail entire project export if something goes wrong during file download + # (e.g. downloaded file has filename that exceeds 255 characters). # Ignore raised exception, skip such upload, log the error and keep going with the export instead. Gitlab::ErrorTracking.log_exception(e, portable_id: portable.id, portable_class: portable.class.name, upload_id: upload.id) end diff --git a/app/services/ci/after_requeue_job_service.rb b/app/services/ci/after_requeue_job_service.rb index 634c547a623..9d54207d75d 100644 --- a/app/services/ci/after_requeue_job_service.rb +++ b/app/services/ci/after_requeue_job_service.rb @@ -26,7 +26,7 @@ module Ci return legacy_dependent_jobs unless ::Feature.enabled?(:ci_requeue_with_dag_object_hierarchy, project) ordered_by_dag( - ::Ci::Processable + @processable.pipeline.processables .from_union(needs_dependent_jobs, stage_dependent_jobs) .skipped .ordered_by_stage diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index af175b8da1c..0b49beffcb5 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -26,6 +26,7 @@ module Ci Gitlab::Ci::Pipeline::Chain::AssignPartition, Gitlab::Ci::Pipeline::Chain::Seed, Gitlab::Ci::Pipeline::Chain::Limit::Size, + Gitlab::Ci::Pipeline::Chain::Limit::ActiveJobs, Gitlab::Ci::Pipeline::Chain::Limit::Deployments, Gitlab::Ci::Pipeline::Chain::Validate::External, Gitlab::Ci::Pipeline::Chain::Populate, @@ -36,7 +37,6 @@ module Ci Gitlab::Ci::Pipeline::Chain::CreateDeployments, Gitlab::Ci::Pipeline::Chain::CreateCrossDatabaseAssociations, Gitlab::Ci::Pipeline::Chain::Limit::Activity, - Gitlab::Ci::Pipeline::Chain::Limit::JobActivity, Gitlab::Ci::Pipeline::Chain::CancelPendingPipelines, Gitlab::Ci::Pipeline::Chain::Metrics, Gitlab::Ci::Pipeline::Chain::TemplateUsage, @@ -140,7 +140,7 @@ module Ci end def create_namespace_onboarding_action - Namespaces::OnboardingPipelineCreatedWorker.perform_async(project.namespace_id) + Onboarding::PipelineCreatedWorker.perform_async(project.namespace_id) end def extra_options(content: nil, dry_run: false) diff --git a/app/services/ci/generate_kubeconfig_service.rb b/app/services/ci/generate_kubeconfig_service.rb index 894ab8e8505..347bc99dbf5 100644 --- a/app/services/ci/generate_kubeconfig_service.rb +++ b/app/services/ci/generate_kubeconfig_service.rb @@ -14,7 +14,8 @@ module Ci url: Gitlab::Kas.tunnel_url ) - agents.each do |agent| + agent_authorizations.each do |authorization| + agent = authorization.agent user = user_name(agent) template.add_user( @@ -24,6 +25,7 @@ module Ci template.add_context( name: context_name(agent), + namespace: context_namespace(authorization), cluster: cluster_name, user: user ) @@ -36,8 +38,8 @@ module Ci attr_reader :pipeline, :token, :template - def agents - pipeline.authorized_cluster_agents + def agent_authorizations + pipeline.cluster_agent_authorizations end def cluster_name @@ -52,6 +54,10 @@ module Ci [agent.project.full_path, agent.name].join(delimiter) end + def context_namespace(authorization) + authorization.config['default_namespace'] + end + def agent_token(agent) ['ci', agent.id, token].join(delimiter) end diff --git a/app/services/ci/job_artifacts/delete_service.rb b/app/services/ci/job_artifacts/delete_service.rb index 65cae03312e..c9d590eccc4 100644 --- a/app/services/ci/job_artifacts/delete_service.rb +++ b/app/services/ci/job_artifacts/delete_service.rb @@ -15,13 +15,23 @@ module Ci method: 'Ci::JobArtifacts::DeleteService#execute', project_id: build.project_id ) + return ServiceResponse.error( + message: 'Action temporarily disabled. The project this job belongs to is undergoing stats refresh.', + reason: :project_stats_refresh + ) end - # fix_expire_at is false because in this case we want to explicitly delete the job artifacts - # this flag is a workaround that will be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/355833 - Ci::JobArtifacts::DestroyBatchService.new(build.job_artifacts.erasable, fix_expire_at: false).execute + result = Ci::JobArtifacts::DestroyBatchService.new(build.job_artifacts.erasable).execute - ServiceResponse.success + if result.fetch(:status) == :success + ServiceResponse.success(payload: + { + destroyed_artifacts_count: result.fetch(:destroyed_artifacts_count), + statistics_updates: result.fetch(:statistics_updates) + }) + else + ServiceResponse.error(message: result.fetch(:message)) + end end private diff --git a/app/services/ci/parse_dotenv_artifact_service.rb b/app/services/ci/parse_dotenv_artifact_service.rb index fd13ed245cf..14e8dc41cf5 100644 --- a/app/services/ci/parse_dotenv_artifact_service.rb +++ b/app/services/ci/parse_dotenv_artifact_service.rb @@ -40,7 +40,7 @@ module Ci key, value = scan_line!(line) variables[key] = Ci::JobVariable.new(job_id: artifact.job_id, - source: :dotenv, key: key, value: value) + source: :dotenv, key: key, value: value, raw: false) end end diff --git a/app/services/ci/pipeline_artifacts/coverage_report_service.rb b/app/services/ci/pipeline_artifacts/coverage_report_service.rb index 99877603554..9c6fdb7a405 100644 --- a/app/services/ci/pipeline_artifacts/coverage_report_service.rb +++ b/app/services/ci/pipeline_artifacts/coverage_report_service.rb @@ -27,18 +27,13 @@ module Ci end def pipeline_artifact_params - attributes = { + { pipeline: pipeline, file_type: :code_coverage, file: carrierwave_file, - size: carrierwave_file['tempfile'].size + size: carrierwave_file['tempfile'].size, + locked: pipeline.locked } - - if ::Feature.enabled?(:ci_update_unlocked_pipeline_artifacts, pipeline.project) - attributes[:locked] = pipeline.locked - end - - attributes end def carrierwave_file diff --git a/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb b/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb index aeb68a75f88..a0746ef32b2 100644 --- a/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb +++ b/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb @@ -23,20 +23,15 @@ module Ci def artifact_attributes file = build_carrierwave_file! - attributes = { + { project_id: pipeline.project_id, file_type: :code_quality_mr_diff, file_format: Ci::PipelineArtifact::REPORT_TYPES.fetch(:code_quality_mr_diff), size: file["tempfile"].size, file: file, - expire_at: Ci::PipelineArtifact::EXPIRATION_DATE.from_now + expire_at: Ci::PipelineArtifact::EXPIRATION_DATE.from_now, + locked: pipeline.locked } - - if ::Feature.enabled?(:ci_update_unlocked_pipeline_artifacts, pipeline.project) - attributes[:locked] = pipeline.locked - end - - attributes end def merge_requests diff --git a/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb b/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb index 17c039885e5..8dddf3c3f6c 100644 --- a/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb +++ b/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb @@ -3,20 +3,26 @@ module Ci module PipelineArtifacts class DestroyAllExpiredService + include ::Gitlab::ExclusiveLeaseHelpers include ::Gitlab::LoopHelpers include ::Gitlab::Utils::StrongMemoize BATCH_SIZE = 100 - LOOP_TIMEOUT = 5.minutes LOOP_LIMIT = 1000 + LOOP_TIMEOUT = 5.minutes + LOCK_TIMEOUT = 10.minutes + EXCLUSIVE_LOCK_KEY = 'expired_pipeline_artifacts:destroy:lock' def initialize @removed_artifacts_count = 0 + @start_at = Time.current end def execute - loop_until(timeout: LOOP_TIMEOUT, limit: LOOP_LIMIT) do - destroy_artifacts_batch + in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do + destroy_unlocked_pipeline_artifacts + + legacy_destroy_pipeline_artifacts end @removed_artifacts_count @@ -24,10 +30,30 @@ module Ci private + def destroy_unlocked_pipeline_artifacts + loop_until(timeout: LOOP_TIMEOUT, limit: LOOP_LIMIT) do + artifacts = Ci::PipelineArtifact.expired_before(@start_at).artifact_unlocked.limit(BATCH_SIZE) + + break if artifacts.empty? + + destroy_batch(artifacts) + end + end + + def legacy_destroy_pipeline_artifacts + loop_until(timeout: LOOP_TIMEOUT, limit: LOOP_LIMIT) do + destroy_artifacts_batch + end + end + def destroy_artifacts_batch artifacts = ::Ci::PipelineArtifact.unlocked.expired.limit(BATCH_SIZE).to_a return false if artifacts.empty? + destroy_batch(artifacts) + end + + def destroy_batch(artifacts) artifacts.each(&:destroy!) increment_stats(artifacts.size) diff --git a/app/services/ci/process_build_service.rb b/app/services/ci/process_build_service.rb index e6ec65fcc91..22cd267806d 100644 --- a/app/services/ci/process_build_service.rb +++ b/app/services/ci/process_build_service.rb @@ -25,6 +25,8 @@ module Ci end def enqueue(build) + return build.drop!(:failed_outdated_deployment_job) if build.prevent_rollback_deployment? + build.enqueue end diff --git a/app/services/ci/runners/register_runner_service.rb b/app/services/ci/runners/register_runner_service.rb index ae9b8bc8a16..abd32610cec 100644 --- a/app/services/ci/runners/register_runner_service.rb +++ b/app/services/ci/runners/register_runner_service.rb @@ -59,7 +59,7 @@ module Ci end def runner_registrar_valid?(type) - Feature.disabled?(:runner_registration_control) || Gitlab::CurrentSettings.valid_runner_registrars.include?(type) + Gitlab::CurrentSettings.valid_runner_registrars.include?(type) end def token_scope diff --git a/app/services/ci/unlock_artifacts_service.rb b/app/services/ci/unlock_artifacts_service.rb index 1fee31da4fc..574cdae6480 100644 --- a/app/services/ci/unlock_artifacts_service.rb +++ b/app/services/ci/unlock_artifacts_service.rb @@ -11,8 +11,6 @@ module Ci unlocked_pipeline_artifacts: 0 } - unlock_pipeline_artifacts_enabled = ::Feature.enabled?(:ci_update_unlocked_pipeline_artifacts, ci_ref.project) - if ::Feature.enabled?(:ci_update_unlocked_job_artifacts, ci_ref.project) loop do unlocked_pipelines = [] @@ -22,9 +20,7 @@ module Ci unlocked_pipelines = unlock_pipelines(ci_ref, before_pipeline) unlocked_job_artifacts = unlock_job_artifacts(unlocked_pipelines) - if unlock_pipeline_artifacts_enabled - results[:unlocked_pipeline_artifacts] += unlock_pipeline_artifacts(unlocked_pipelines) - end + results[:unlocked_pipeline_artifacts] += unlock_pipeline_artifacts(unlocked_pipelines) end break if unlocked_pipelines.empty? diff --git a/app/services/clusters/applications/destroy_service.rb b/app/services/clusters/applications/destroy_service.rb deleted file mode 100644 index d666682487b..00000000000 --- a/app/services/clusters/applications/destroy_service.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -module Clusters - module Applications - class DestroyService < ::Clusters::Applications::BaseService - def execute(_request) - instantiate_application.tap do |application| - break unless application.can_uninstall? - - application.make_scheduled! - - Clusters::Applications::UninstallWorker.perform_async(application.name, application.id) - end - end - - private - - def builder - cluster.public_send(application_class.association_name) # rubocop:disable GitlabSecurity/PublicSend - end - end - end -end diff --git a/app/services/clusters/applications/uninstall_service.rb b/app/services/clusters/applications/uninstall_service.rb deleted file mode 100644 index 50c8d806c14..00000000000 --- a/app/services/clusters/applications/uninstall_service.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -module Clusters - module Applications - class UninstallService < BaseHelmService - def execute - return unless app.scheduled? - - app.make_uninstalling! - uninstall - end - - private - - def uninstall - helm_api.uninstall(app.uninstall_command) - - Clusters::Applications::WaitForUninstallAppWorker.perform_in( - Clusters::Applications::WaitForUninstallAppWorker::INTERVAL, app.name, app.id) - rescue Kubeclient::HttpError => e - log_error(e) - app.make_errored!("Kubernetes error: #{e.error_code}") - rescue StandardError => e - log_error(e) - app.make_errored!('Failed to uninstall.') - end - end - end -end diff --git a/app/services/concerns/users/participable_service.rb b/app/services/concerns/users/participable_service.rb index c1c93aa604e..281b2508090 100644 --- a/app/services/concerns/users/participable_service.rb +++ b/app/services/concerns/users/participable_service.rb @@ -32,6 +32,8 @@ module Users end def groups + return [] unless current_user + current_user.authorized_groups.with_route.sort_by(&:path) end diff --git a/app/services/concerns/work_items/widgetable_service.rb b/app/services/concerns/work_items/widgetable_service.rb index beb614c7b76..24ade9336b2 100644 --- a/app/services/concerns/work_items/widgetable_service.rb +++ b/app/services/concerns/work_items/widgetable_service.rb @@ -2,18 +2,22 @@ module WorkItems module WidgetableService - def execute_widgets(work_item:, callback:, widget_params: {}) + def execute_widgets(work_item:, callback:, widget_params: {}, service_params: {}) work_item.widgets.each do |widget| - widget_service(widget).try(callback, params: widget_params[widget.class.api_symbol]) + widget_service(widget, service_params).try(callback, params: widget_params[widget.class.api_symbol]) end end # rubocop:disable Gitlab/ModuleWithInstanceVariables - def widget_service(widget) + def widget_service(widget, service_params) @widget_services ||= {} return @widget_services[widget] if @widget_services.has_key?(widget) - @widget_services[widget] = widget_service_class(widget)&.new(widget: widget, current_user: current_user) + @widget_services[widget] = widget_service_class(widget)&.new( + widget: widget, + current_user: current_user, + service_params: service_params + ) end # rubocop:enable Gitlab/ModuleWithInstanceVariables diff --git a/app/services/google_cloud/enable_cloudsql_service.rb b/app/services/google_cloud/enable_cloudsql_service.rb index e4a411d0fab..911cccca5ca 100644 --- a/app/services/google_cloud/enable_cloudsql_service.rb +++ b/app/services/google_cloud/enable_cloudsql_service.rb @@ -3,7 +3,7 @@ module GoogleCloud class EnableCloudsqlService < ::GoogleCloud::BaseService def execute - return no_projects_error if unique_gcp_project_ids.empty? + create_or_replace_project_vars(environment_name, 'GCP_PROJECT_ID', gcp_project_id, ci_var_protected?) unique_gcp_project_ids.each do |gcp_project_id| google_api_client.enable_cloud_sql_admin(gcp_project_id) @@ -18,8 +18,8 @@ module GoogleCloud private - def no_projects_error - error("No GCP projects found. Configure a service account or GCP_PROJECT_ID CI variable.") + def ci_var_protected? + ProtectedBranch.protected?(project, environment_name) || ProtectedTag.protected?(project, environment_name) end end end diff --git a/app/services/groups/import_export/import_service.rb b/app/services/groups/import_export/import_service.rb index db52a272bf2..4092ded67bc 100644 --- a/app/services/groups/import_export/import_service.rb +++ b/app/services/groups/import_export/import_service.rb @@ -26,6 +26,8 @@ module Groups end def execute + Gitlab::Tracking.event(self.class.name, 'create', label: 'import_group_from_file') + if valid_user_permissions? && import_file && restorers.all?(&:restore) notify_success diff --git a/app/services/import/github/cancel_project_import_service.rb b/app/services/import/github/cancel_project_import_service.rb new file mode 100644 index 00000000000..5dce5e73662 --- /dev/null +++ b/app/services/import/github/cancel_project_import_service.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Import + module Github + class CancelProjectImportService < ::BaseService + def execute + return error('Not Found', :not_found) unless authorized_to_read? + return error('Unauthorized access', :forbidden) unless authorized_to_cancel? + + if project.import_in_progress? + project.import_state.cancel + success(project: project) + else + error(cannot_cancel_error_message, :bad_request) + end + end + + private + + def authorized_to_read? + can?(current_user, :read_project, project) + end + + def authorized_to_cancel? + can?(current_user, :owner_access, project) + end + + def cannot_cancel_error_message + format( + _('The import cannot be canceled because it is %{project_status}'), + project_status: project.import_state.status + ) + end + end + end +end diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index 53297d2412c..a60963e28c7 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -9,21 +9,13 @@ module Import attr_reader :params, :current_user def execute(access_params, provider) - if blocked_url? - return log_and_return_error("Invalid URL: #{url}", _("Invalid URL: %{url}") % { url: url }, :bad_request) - end - - unless authorized? - return error(_('This namespace has already been taken! Please choose another one.'), :unprocessable_entity) - end - - if oversized? - return error(oversize_error_message, :unprocessable_entity) - end + context_error = validate_context + return context_error if context_error project = create_project(access_params, provider) if project.persisted? + store_import_settings(project) success(project) elsif project.errors[:import_source_disabled].present? error(project.errors[:import_source_disabled], :forbidden) @@ -108,6 +100,16 @@ module Import private + def validate_context + if blocked_url? + log_and_return_error("Invalid URL: #{url}", _("Invalid URL: %{url}") % { url: url }, :bad_request) + elsif !authorized? + error(_('This namespace has already been taken. Choose a different one.'), :unprocessable_entity) + elsif oversized? + error(oversize_error_message, :unprocessable_entity) + end + end + def log_error(exception) Gitlab::GithubImport::Logger.error( message: 'Import failed due to a GitHub error', @@ -126,6 +128,10 @@ module Import error(translated_message, http_status) end + + def store_import_settings(project) + Gitlab::GithubImport::Settings.new(project).write(params[:optional_stages]) + end end end diff --git a/app/services/incident_management/incidents/create_service.rb b/app/services/incident_management/incidents/create_service.rb index ef66325fdcc..f44842650b7 100644 --- a/app/services/incident_management/incidents/create_service.rb +++ b/app/services/incident_management/incidents/create_service.rb @@ -15,7 +15,7 @@ module IncidentManagement end def execute - issue = Issues::CreateService.new( + create_result = Issues::CreateService.new( project: project, current_user: current_user, params: { @@ -29,22 +29,16 @@ module IncidentManagement ).execute if alert - return error(alert.errors.full_messages.to_sentence, issue) unless alert.valid? + return error(alert.errors.full_messages, create_result[:issue]) unless alert.valid? end - return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid? - - success(issue) + create_result end private attr_reader :title, :description, :severity, :alert - def success(issue) - ServiceResponse.success(payload: { issue: issue }) - end - def error(message, issue = nil) ServiceResponse.error(payload: { issue: issue }, message: message) 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 58777848151..d495ec5cab6 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 @@ -11,7 +11,7 @@ module IncidentManagement @issuable = issuable @param_errors = [] - super(project: issuable.project, current_user: current_user, params: Hash(params)) + super(project: issuable.project, current_user: current_user, params: params) end def execute diff --git a/app/services/incident_management/timeline_events/create_service.rb b/app/services/incident_management/timeline_events/create_service.rb index 40ce9097c88..5422b4ad6d2 100644 --- a/app/services/incident_management/timeline_events/create_service.rb +++ b/app/services/incident_management/timeline_events/create_service.rb @@ -109,7 +109,6 @@ module IncidentManagement def add_system_note(timeline_event) return if auto_created - return unless Feature.enabled?(:incident_timeline, project) SystemNoteService.add_timeline_event(timeline_event) end diff --git a/app/services/incident_management/timeline_events/destroy_service.rb b/app/services/incident_management/timeline_events/destroy_service.rb index 90e95ae8869..e1c6bbbdb85 100644 --- a/app/services/incident_management/timeline_events/destroy_service.rb +++ b/app/services/incident_management/timeline_events/destroy_service.rb @@ -30,8 +30,6 @@ module IncidentManagement attr_reader :project, :timeline_event, :user, :incident def add_system_note(incident, user) - return unless Feature.enabled?(:incident_timeline, project) - SystemNoteService.delete_timeline_event(incident, user) end end diff --git a/app/services/incident_management/timeline_events/update_service.rb b/app/services/incident_management/timeline_events/update_service.rb index 5c5de4717bc..012e2f0e260 100644 --- a/app/services/incident_management/timeline_events/update_service.rb +++ b/app/services/incident_management/timeline_events/update_service.rb @@ -38,8 +38,6 @@ module IncidentManagement end def add_system_note(timeline_event) - return unless Feature.enabled?(:incident_timeline, incident.project) - changes = was_changed(timeline_event) return if changes == :none diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb index 822e3cd787c..e84d1032e41 100644 --- a/app/services/issuable/import_csv/base_service.rb +++ b/app/services/issuable/import_csv/base_service.rb @@ -23,7 +23,7 @@ module Issuable with_csv_lines.each do |row, line_no| attributes = issuable_attributes_for(row) - if create_issuable(attributes).persisted? + if create_issuable(attributes)&.persisted? @results[:success] += 1 else @results[:error_lines].push(line_no) diff --git a/app/services/issuable/process_assignees.rb b/app/services/issuable/process_assignees.rb index 1ef6d3d9c42..72f727c134d 100644 --- a/app/services/issuable/process_assignees.rb +++ b/app/services/issuable/process_assignees.rb @@ -6,11 +6,11 @@ module Issuable class ProcessAssignees def initialize(assignee_ids:, add_assignee_ids:, remove_assignee_ids:, existing_assignee_ids: nil, extra_assignee_ids: nil) - @assignee_ids = assignee_ids - @add_assignee_ids = add_assignee_ids - @remove_assignee_ids = remove_assignee_ids - @existing_assignee_ids = existing_assignee_ids || [] - @extra_assignee_ids = extra_assignee_ids || [] + @assignee_ids = assignee_ids&.map(&:to_i) + @add_assignee_ids = add_assignee_ids&.map(&:to_i) + @remove_assignee_ids = remove_assignee_ids&.map(&:to_i) + @existing_assignee_ids = existing_assignee_ids&.map(&:to_i) || [] + @extra_assignee_ids = extra_assignee_ids&.map(&:to_i) || [] end def execute diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 70ad97f8436..e24ae8f59f0 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -67,22 +67,14 @@ class IssuableBaseService < ::BaseProjectService end def filter_assignees(issuable) - filter_assignees_with_key(issuable, :assignee_ids, :assignees) - filter_assignees_with_key(issuable, :add_assignee_ids, :add_assignees) - filter_assignees_with_key(issuable, :remove_assignee_ids, :remove_assignees) + filter_assignees_using_checks(issuable, :assignee_ids) + filter_assignees_using_checks(issuable, :add_assignee_ids) + filter_assignees_using_checks(issuable, :remove_assignee_ids) end - def filter_assignees_with_key(issuable, id_key, key) - if params[key] && params[id_key].blank? - params[id_key] = params[key].map(&:id) - end - + def filter_assignees_using_checks(issuable, id_key) return if params[id_key].blank? - filter_assignees_using_checks(issuable, id_key) - end - - def filter_assignees_using_checks(issuable, id_key) unless issuable.allows_multiple_assignees? params[id_key] = params[id_key].first(1) end @@ -154,10 +146,13 @@ class IssuableBaseService < ::BaseProjectService end def filter_escalation_status(issuable) + status_params = params.delete(:escalation_status) || {} + status_params.permit! if status_params.respond_to?(:permit!) + result = ::IncidentManagement::IssuableEscalationStatuses::PrepareUpdateService.new( issuable, current_user, - params.delete(:escalation_status) + status_params ).execute return unless result.success? && result[:escalation_status].present? @@ -266,11 +261,23 @@ class IssuableBaseService < ::BaseProjectService # To be overridden by subclasses end - def after_update(issuable) + def prepare_update_params(issuable) # To be overridden by subclasses end + def after_update(issuable, old_associations) + handle_description_updated(issuable) + handle_label_changes(issuable, old_associations[:labels]) + end + + def handle_description_updated(issuable) + return unless issuable.previous_changes.include?('description') + + GraphqlTriggers.issuable_description_updated(issuable) + end + def update(issuable) + prepare_update_params(issuable) handle_quick_actions(issuable) filter_params(issuable) @@ -316,7 +323,7 @@ class IssuableBaseService < ::BaseProjectService affected_assignees = (old_associations[:assignees] + new_assignees) - (old_associations[:assignees] & new_assignees) invalidate_cache_counts(issuable, users: affected_assignees.compact) - after_update(issuable) + after_update(issuable, old_associations) issuable.create_new_cross_references!(current_user) execute_hooks( issuable, @@ -356,7 +363,8 @@ class IssuableBaseService < ::BaseProjectService handle_task_changes(issuable) invalidate_cache_counts(issuable, users: issuable.assignees.to_a) - after_update(issuable) + # not passing old_associations here to keep `update_task` as fast as possible + after_update(issuable, {}) execute_hooks(issuable, 'update', old_associations: nil) if issuable.is_a?(MergeRequest) @@ -531,6 +539,8 @@ class IssuableBaseService < ::BaseProjectService end def has_label_changes?(issuable, old_labels) + return false if old_labels.nil? + Set.new(issuable.labels) != Set.new(old_labels) end @@ -542,12 +552,15 @@ class IssuableBaseService < ::BaseProjectService # override if needed def handle_label_changes(issuable, old_labels) - return unless has_label_changes?(issuable, old_labels) + return false unless has_label_changes?(issuable, old_labels) # reset to preserve the label sort order (title ASC) issuable.labels.reset GraphqlTriggers.issuable_labels_updated(issuable) + + # return true here to avoid checking for label changes in sub classes + true end # override if needed diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index d75e74f3b19..28ea6b0ebf8 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -68,6 +68,19 @@ module Issues rebalance_if_needed(issue) end + def handle_escalation_status_change(issue) + return unless issue.supports_escalation? + + if issue.escalation_status + ::IncidentManagement::IssuableEscalationStatuses::AfterUpdateService.new( + issue, + current_user + ).execute + else + ::IncidentManagement::IssuableEscalationStatuses::CreateService.new(issue).execute + end + end + def issuable_for_positioning(id, positioning_scope) return unless id diff --git a/app/services/issues/clone_service.rb b/app/services/issues/clone_service.rb index 07dd9a98f89..8b05a1c2acd 100644 --- a/app/services/issues/clone_service.rb +++ b/app/services/issues/clone_service.rb @@ -75,7 +75,16 @@ module Issues # Skip creation of system notes for existing attributes of the issue when cloning with notes. # The system notes of the old issue are copied over so we don't want to end up with duplicate notes. # When cloning without notes, we want to generate system notes for the attributes that were copied. - CreateService.new(project: target_project, current_user: current_user, params: new_params, spam_params: spam_params).execute(skip_system_notes: with_notes) + create_result = CreateService.new( + project: target_project, + current_user: current_user, + params: new_params, + spam_params: spam_params + ).execute(skip_system_notes: with_notes) + + raise CloneError, create_result.errors.join(', ') if create_result.error? && create_result[:issue].blank? + + create_result[:issue] end def queue_copy_designs diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 92cf4811439..89b35bbab24 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -4,6 +4,7 @@ module Issues class CreateService < Issues::BaseService include ResolveDiscussions prepend RateLimitedService + include ::Services::ReturnServiceResponses rate_limit key: :issues_create, opts: { scope: [:project, :current_user, :external_author] } @@ -20,6 +21,8 @@ module Issues end def execute(skip_system_notes: false) + return error(_('Operation not allowed'), 403) unless @current_user.can?(authorization_action, @project) + @issue = @build_service.execute handle_move_between_ids(@issue) @@ -27,7 +30,13 @@ module Issues @add_related_issue ||= params.delete(:add_related_issue) filter_resolve_discussion_params - create(@issue, skip_system_notes: skip_system_notes) + issue = create(@issue, skip_system_notes: skip_system_notes) + + if issue.persisted? + success(issue: issue) + else + error(issue.errors.full_messages, 422, pass_back: { issue: issue }) + end end def external_author @@ -47,7 +56,7 @@ module Issues issue.run_after_commit do NewIssueWorker.perform_async(issue.id, user.id, issue.class.to_s) Issues::PlacementWorker.perform_async(nil, issue.project_id) - Namespaces::OnboardingIssueCreatedWorker.perform_async(issue.project.namespace_id) + Onboarding::IssueCreatedWorker.perform_async(issue.project.namespace_id) end end @@ -56,7 +65,7 @@ module Issues user_agent_detail_service.create handle_add_related_issue(issue) resolve_discussions_with_issue(issue) - create_escalation_status(issue) + handle_escalation_status_change(issue) create_timeline_event(issue) try_to_associate_contacts(issue) @@ -87,12 +96,12 @@ module Issues private - attr_reader :spam_params, :extra_params - - def create_escalation_status(issue) - ::IncidentManagement::IssuableEscalationStatuses::CreateService.new(issue).execute if issue.supports_escalation? + def authorization_action + :create_issue end + attr_reader :spam_params, :extra_params + def create_timeline_event(issue) return unless issue.incident? diff --git a/app/services/issues/import_csv_service.rb b/app/services/issues/import_csv_service.rb index bce3ecc8bef..83e550583f6 100644 --- a/app/services/issues/import_csv_service.rb +++ b/app/services/issues/import_csv_service.rb @@ -14,6 +14,10 @@ module Issues private + def create_issuable(attributes) + super[:issue] + end + def create_issuable_class Issues::CreateService end diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index edab62b1fdf..6366ff4076b 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -83,7 +83,16 @@ module Issues # Skip creation of system notes for existing attributes of the issue. The system notes of the old # issue are copied over so we don't want to end up with duplicate notes. - CreateService.new(project: @target_project, current_user: @current_user, params: new_params, spam_params: spam_params).execute(skip_system_notes: true) + create_result = CreateService.new( + project: @target_project, + current_user: @current_user, + params: new_params, + spam_params: spam_params + ).execute(skip_system_notes: true) + + raise MoveError, create_result.errors.join(', ') if create_result.error? && create_result[:issue].blank? + + create_result[:issue] end def queue_copy_designs diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 46c28d82ddc..e5feb4422f6 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -63,7 +63,6 @@ module Issues handle_assignee_changes(issue, old_assignees) handle_confidential_change(issue) - handle_label_changes(issue, old_labels) handle_added_labels(issue, old_labels) handle_milestone_change(issue) handle_added_mentions(issue, old_mentioned_users) @@ -201,15 +200,6 @@ module Issues ::IncidentManagement::AddSeveritySystemNoteWorker.perform_async(issue.id, current_user.id) end - def handle_escalation_status_change(issue) - return unless issue.supports_escalation? && issue.escalation_status - - ::IncidentManagement::IssuableEscalationStatuses::AfterUpdateService.new( - issue, - current_user - ).execute - end - def create_confidentiality_note(issue) SystemNoteService.change_issue_confidentiality(issue, issue.project, current_user) end diff --git a/app/services/jira_connect/create_asymmetric_jwt_service.rb b/app/services/jira_connect/create_asymmetric_jwt_service.rb new file mode 100644 index 00000000000..71aba6feddd --- /dev/null +++ b/app/services/jira_connect/create_asymmetric_jwt_service.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module JiraConnect + class CreateAsymmetricJwtService + ARGUMENT_ERROR_MESSAGE = 'jira_connect_installation is not a proxy installation' + + def initialize(jira_connect_installation) + raise ArgumentError, ARGUMENT_ERROR_MESSAGE unless jira_connect_installation.proxy? + + @jira_connect_installation = jira_connect_installation + end + + def execute + JWT.encode(jwt_claims, private_key, 'RS256', jwt_headers) + end + + private + + def jwt_claims + { aud: aud_claim, iss: iss_claim, qsh: qsh_claim } + end + + def aud_claim + @jira_connect_installation.audience_url + end + + def iss_claim + @jira_connect_installation.client_key + end + + def qsh_claim + Atlassian::Jwt.create_query_string_hash( + @jira_connect_installation.audience_installed_event_url, + 'POST', + @jira_connect_installation.audience_url + ) + end + + def private_key + @private_key ||= OpenSSL::PKey::RSA.generate(3072) + end + + def public_key_storage + @public_key_storage ||= JiraConnect::PublicKey.create!(key: private_key.public_key) + end + + def jwt_headers + { kid: public_key_storage.uuid } + end + end +end diff --git a/app/services/labels/promote_service.rb b/app/services/labels/promote_service.rb index e3b110f8f26..2786a2e357e 100644 --- a/app/services/labels/promote_service.rb +++ b/app/services/labels/promote_service.rb @@ -9,7 +9,7 @@ module Labels return unless project.group && label.is_a?(ProjectLabel) - Label.transaction do + ProjectLabel.transaction do # use the existing group label if it exists group_label = find_or_create_group_label(label) @@ -50,7 +50,7 @@ module Labels .new(current_user, title: group_label.title, group_id: project.group.id) .execute(skip_authorization: true) .where.not(id: group_label) - .select(:id) # Can't use pluck() to avoid object-creation because of the batching + .select(:id, :project_id, :group_id, :type) # Can't use pluck() to avoid object-creation because of the batching end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 38bebc1d09d..aba075c3644 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -179,7 +179,7 @@ module Members def enqueue_onboarding_progress_action return unless member_created_namespace_id - Namespaces::OnboardingUserAddedWorker.perform_async(member_created_namespace_id) + Onboarding::UserAddedWorker.perform_async(member_created_namespace_id) end def result @@ -195,6 +195,8 @@ module Members end def publish_event! + return unless member_created_namespace_id + Gitlab::EventStore.publish( Members::MembersAddedEvent.new(data: { source_id: source.id, diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 0a8344c58db..ce79907e8a8 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -2,6 +2,8 @@ module Members class DestroyService < Members::BaseService + include Gitlab::ExclusiveLeaseHelpers + def execute(member, skip_authorization: false, skip_subresources: false, unassign_issuables: false, destroy_bot: false) unless skip_authorization raise Gitlab::Access::AccessDeniedError unless authorized?(member, destroy_bot) @@ -11,13 +13,26 @@ module Members end @skip_auth = skip_authorization + last_owner = true + + in_lock("delete_members:#{member.source.class}:#{member.source.id}") do + break if member.is_a?(GroupMember) && member.source.last_owner?(member.user) + + last_owner = false + member.destroy + member.user&.invalidate_cache_counts + end - return member if member.is_a?(GroupMember) && member.source.last_owner?(member.user) + unless last_owner + delete_member_associations(member, skip_subresources, unassign_issuables) + end - member.destroy + member + end - member.user&.invalidate_cache_counts + private + def delete_member_associations(member, skip_subresources, unassign_issuables) if member.request? && member.user != current_user notification_service.decline_access_request(member) end @@ -28,12 +43,8 @@ module Members enqueue_unassign_issuables(member) if unassign_issuables after_execute(member: member) - - member end - private - def authorized?(member, destroy_bot) return can_destroy_bot_member?(member) if destroy_bot diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb index 64ae33c9b15..5761e34caff 100644 --- a/app/services/merge_requests/approval_service.rb +++ b/app/services/merge_requests/approval_service.rb @@ -3,7 +3,7 @@ module MergeRequests class ApprovalService < MergeRequests::BaseService def execute(merge_request) - return unless can_be_approved?(merge_request) + return unless eligible_for_approval?(merge_request) approval = merge_request.approvals.new(user: current_user) @@ -28,8 +28,8 @@ module MergeRequests private - def can_be_approved?(merge_request) - merge_request.can_be_approved_by?(current_user) + def eligible_for_approval?(merge_request) + merge_request.eligible_for_approval_by?(current_user) end def save_approval(approval) diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 6cefd9169f5..cfd7c645b7e 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -58,6 +58,7 @@ module MergeRequests new_reviewers = merge_request.reviewers - old_reviewers merge_request_activity_counter.track_users_review_requested(users: new_reviewers) merge_request_activity_counter.track_reviewers_changed_action(user: current_user) + trigger_merge_request_reviewers_updated(merge_request) end def cleanup_environments(merge_request) @@ -244,6 +245,10 @@ module MergeRequests Milestones::MergeRequestsCountService.new(milestone).delete_cache end + + def trigger_merge_request_reviewers_updated(merge_request) + GraphqlTriggers.merge_request_reviewers_updated(merge_request) + end end end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index f83b14c7269..da3a9652d69 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -23,6 +23,7 @@ module MergeRequests cleanup_environments(merge_request) abort_auto_merge(merge_request, 'merge request was closed') cleanup_refs(merge_request) + trigger_merge_request_merge_status_updated(merge_request) end merge_request @@ -38,5 +39,9 @@ module MergeRequests merge_request_metrics_service(merge_request).close(close_event) end end + + def trigger_merge_request_merge_status_updated(merge_request) + GraphqlTriggers.merge_request_merge_status_updated(merge_request) + end end end diff --git a/app/services/merge_requests/mark_reviewer_reviewed_service.rb b/app/services/merge_requests/mark_reviewer_reviewed_service.rb index 766a4ca0a49..96747eabcf6 100644 --- a/app/services/merge_requests/mark_reviewer_reviewed_service.rb +++ b/app/services/merge_requests/mark_reviewer_reviewed_service.rb @@ -10,6 +10,8 @@ module MergeRequests if reviewer return error("Failed to update reviewer") unless reviewer.update(state: :reviewed) + trigger_merge_request_reviewers_updated(merge_request) + success else error("Reviewer not found") diff --git a/app/services/merge_requests/merge_base_service.rb b/app/services/merge_requests/merge_base_service.rb index 3e630d40b3d..2a3c1e8bc26 100644 --- a/app/services/merge_requests/merge_base_service.rb +++ b/app/services/merge_requests/merge_base_service.rb @@ -9,12 +9,12 @@ module MergeRequests attr_reader :merge_request # Overridden in EE. - def hooks_validation_pass?(_merge_request) + def hooks_validation_pass?(merge_request, validate_squash_message: false) true end # Overridden in EE. - def hooks_validation_error(_merge_request) + def hooks_validation_error(merge_request, validate_squash_message: false) # No-op end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 6d31a29f5a7..6b4f9dbe509 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -26,6 +26,7 @@ module MergeRequests @merge_request = merge_request @options = options + jid = merge_jid validate! @@ -37,7 +38,7 @@ module MergeRequests end end - log_info("Merge process finished on JID #{merge_jid} with state #{state}") + log_info("Merge process finished on JID #{jid} with state #{state}") rescue MergeError => e handle_merge_error(log_message: e.message, save_message_on_model: true) ensure @@ -159,17 +160,32 @@ module MergeRequests end def handle_merge_error(log_message:, save_message_on_model: false) - Gitlab::AppLogger.error("MergeService ERROR: #{merge_request_info} - #{log_message}") + log_error("MergeService ERROR: #{merge_request_info} - #{log_message}") @merge_request.update(merge_error: log_message) if save_message_on_model end def log_info(message) + payload = log_payload("#{merge_request_info} - #{message}") + logger.info(**payload) + end + + def log_error(message) + payload = log_payload(message) + logger.error(**payload) + end + + def logger @logger ||= Gitlab::AppLogger - @logger.info("#{merge_request_info} - #{message}") + end + + def log_payload(message) + Gitlab::ApplicationContext.current + .merge(merge_request_info: merge_request_info, + message: message) end def merge_request_info - merge_request.to_reference(full: true) + @merge_request_info ||= merge_request.to_reference(full: true) end def source_matches? diff --git a/app/services/merge_requests/mergeability/logger.rb b/app/services/merge_requests/mergeability/logger.rb index 8b45d231e03..88ef6d81eaa 100644 --- a/app/services/merge_requests/mergeability/logger.rb +++ b/app/services/merge_requests/mergeability/logger.rb @@ -11,16 +11,12 @@ module MergeRequests end def commit - return unless enabled? - commit_logs end def instrument(mergeability_name:) raise ArgumentError, 'block not given' unless block_given? - return yield unless enabled? - op_start_db_counters = current_db_counter_payload op_started_at = current_monotonic_time @@ -38,15 +34,11 @@ module MergeRequests attr_reader :destination, :merge_request def observe(name, value) - return unless enabled? - observations[name.to_s].push(value) end def commit_logs - attributes = Gitlab::ApplicationContext.current.merge({ - mergeability_project_id: merge_request.project.id - }) + attributes = Gitlab::ApplicationContext.current.merge({ mergeability_project_id: merge_request.project.id }) attributes[:mergeability_merge_request_id] = merge_request.id attributes.merge!(observations_hash) @@ -89,12 +81,6 @@ module MergeRequests ::Gitlab::Metrics::Subscribers::ActiveRecord.db_counter_payload end - def enabled? - strong_memoize(:enabled) do - ::Feature.enabled?(:mergeability_checks_logger, merge_request.project) - end - end - def current_monotonic_time ::Gitlab::Metrics::System.monotonic_time end diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb index ef251f121ae..aa52349b0ee 100644 --- a/app/services/merge_requests/push_options_handler_service.rb +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -104,7 +104,7 @@ module MergeRequests merge_request = ::MergeRequests::CreateService.new( project: project, current_user: current_user, - params: merge_request.attributes.merge(assignees: merge_request.assignees, + params: merge_request.attributes.merge(assignee_ids: merge_request.assignee_ids, label_ids: merge_request.label_ids) ).execute end @@ -140,8 +140,8 @@ module MergeRequests params[:add_labels] = params.delete(:label).keys if params.has_key?(:label) params[:remove_labels] = params.delete(:unlabel).keys if params.has_key?(:unlabel) - params[:add_assignee_ids] = params.delete(:assign).keys if params.has_key?(:assign) - params[:remove_assignee_ids] = params.delete(:unassign).keys if params.has_key?(:unassign) + params[:add_assignee_ids] = convert_to_user_ids(params.delete(:assign).keys) if params.has_key?(:assign) + params[:remove_assignee_ids] = convert_to_user_ids(params.delete(:unassign).keys) if params.has_key?(:unassign) if push_options[:milestone] milestone = Milestone.for_projects_and_groups(@project, @project.ancestors_upto)&.find_by_name(push_options[:milestone]) @@ -169,7 +169,7 @@ module MergeRequests params = base_params params.merge!( - assignees: [current_user], + assignee_ids: [current_user.id], source_branch: branch, source_project: project, target_project: target_project @@ -186,6 +186,12 @@ module MergeRequests base_params.merge(merge_params(merge_request.source_branch)) end + def convert_to_user_ids(ids_or_usernames) + ids, usernames = ids_or_usernames.partition { |id_or_username| id_or_username.is_a?(Numeric) || id_or_username.match?(/\A\d+\z/) } + ids += User.by_username(usernames).pluck(:id) unless usernames.empty? # rubocop:disable CodeReuse/ActiveRecord + ids + end + def collect_errors_from_merge_request(merge_request) merge_request.errors.full_messages.each do |error| errors << error diff --git a/app/services/merge_requests/request_review_service.rb b/app/services/merge_requests/request_review_service.rb index b061ed45fee..ebbae98352b 100644 --- a/app/services/merge_requests/request_review_service.rb +++ b/app/services/merge_requests/request_review_service.rb @@ -11,6 +11,7 @@ module MergeRequests return error("Failed to update reviewer") unless reviewer.update(state: :unreviewed) notify_reviewer(merge_request, user) + trigger_merge_request_reviewers_updated(merge_request) success else diff --git a/app/services/merge_requests/update_assignees_service.rb b/app/services/merge_requests/update_assignees_service.rb index a13db52e34b..79a3e9f3c22 100644 --- a/app/services/merge_requests/update_assignees_service.rb +++ b/app/services/merge_requests/update_assignees_service.rb @@ -18,7 +18,17 @@ module MergeRequests return merge_request if old_ids.to_set == new_ids.to_set # no-change attrs = update_attrs.merge(assignee_ids: new_ids) - merge_request.update!(**attrs) + + # We now have assignees validation on merge request + # If we use an update with bang, it will explode, + # instead we need to check if its valid then return if its not valid. + if Feature.enabled?(:limit_assignees_per_issuable) + merge_request.update(**attrs) + + return merge_request unless merge_request.valid? + else + merge_request.update!(**attrs) + end # Defer the more expensive operations (handle_assignee_changes) to the background MergeRequests::HandleAssigneesChangeService diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 6d518edc88f..745647b727c 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -38,7 +38,6 @@ module MergeRequests handle_target_branch_change(merge_request) handle_milestone_change(merge_request) handle_draft_status_change(merge_request, changed_fields) - handle_label_changes(merge_request, old_labels) track_title_and_desc_edits(changed_fields) track_discussion_lock_toggle(merge_request, changed_fields) @@ -71,7 +70,8 @@ module MergeRequests MergeRequests::CloseService end - def after_update(issuable) + def after_update(issuable, old_associations) + super issuable.cache_merge_request_closes_issues!(current_user) end @@ -179,9 +179,12 @@ module MergeRequests old_title_draft = MergeRequest.draft?(old_title) new_title_draft = MergeRequest.draft?(new_title) - # notify the draft status changed. Added/removed message is handled in the - # email template itself, see `change_in_merge_request_draft_status_email` template. - notify_draft_status_changed(merge_request) if old_title_draft || new_title_draft + if old_title_draft || new_title_draft + # notify the draft status changed. Added/removed message is handled in the + # email template itself, see `change_in_merge_request_draft_status_email` template. + notify_draft_status_changed(merge_request) + trigger_merge_request_status_updated(merge_request) + end if !old_title_draft && new_title_draft # Marked as Draft @@ -320,6 +323,10 @@ module MergeRequests def filter_sentinel_values(param) param.reject { _1 == 0 } end + + def trigger_merge_request_status_updated(merge_request) + GraphqlTriggers.merge_request_merge_status_updated(merge_request) + end end end diff --git a/app/services/ml/experiment_tracking/candidate_repository.rb b/app/services/ml/experiment_tracking/candidate_repository.rb new file mode 100644 index 00000000000..b6f87995185 --- /dev/null +++ b/app/services/ml/experiment_tracking/candidate_repository.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module Ml + module ExperimentTracking + class CandidateRepository + attr_accessor :project, :user, :experiment, :candidate + + def initialize(project, user) + @project = project + @user = user + end + + def by_iid(iid) + ::Ml::Candidate.with_project_id_and_iid(project.id, iid) + end + + def create!(experiment, start_time) + experiment.candidates.create!( + user: user, + start_time: start_time || 0 + ) + end + + def update(candidate, status, end_time) + candidate.status = status.downcase if status + candidate.end_time = end_time if end_time + + candidate.save + end + + def add_metric!(candidate, name, value, tracked_at, step) + candidate.metrics.create!( + name: name, + value: value, + tracked_at: tracked_at, + step: step + ) + end + + def add_param!(candidate, name, value) + candidate.params.create!(name: name, value: value) + end + + def add_metrics(candidate, metric_definitions) + return unless candidate.present? + + metrics = metric_definitions.map do |metric| + { + candidate_id: candidate.id, + name: metric[:key], + value: metric[:value], + tracked_at: metric[:timestamp], + step: metric[:step], + **timestamps + } + end + + ::Ml::CandidateMetric.insert_all(metrics, returning: false) unless metrics.empty? + end + + def add_params(candidate, param_definitions) + return unless candidate.present? + + parameters = param_definitions.map do |p| + { + candidate_id: candidate.id, + name: p[:key], + value: p[:value], + **timestamps + } + end + + ::Ml::CandidateParam.insert_all(parameters, returning: false) unless parameters.empty? + end + + private + + def timestamps + current_time = Time.zone.now + + { created_at: current_time, updated_at: current_time } + end + end + end +end diff --git a/app/services/ml/experiment_tracking/experiment_repository.rb b/app/services/ml/experiment_tracking/experiment_repository.rb new file mode 100644 index 00000000000..891674adc2a --- /dev/null +++ b/app/services/ml/experiment_tracking/experiment_repository.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Ml + module ExperimentTracking + class ExperimentRepository + attr_accessor :project, :user + + def initialize(project, user = nil) + @project = project + @user = user + end + + def by_iid_or_name(iid: nil, name: nil) + return ::Ml::Experiment.by_project_id_and_iid(project.id, iid) if iid + + ::Ml::Experiment.by_project_id_and_name(project.id, name) if name + end + + def all + ::Ml::Experiment.by_project_id(project.id) + end + + def create!(name) + ::Ml::Experiment.create!(name: name, + user: user, + project: project) + end + end + end +end diff --git a/app/services/namespaces/package_settings/update_service.rb b/app/services/namespaces/package_settings/update_service.rb index c0af0900450..0c6fcee9113 100644 --- a/app/services/namespaces/package_settings/update_service.rb +++ b/app/services/namespaces/package_settings/update_service.rb @@ -8,7 +8,13 @@ module Namespaces ALLOWED_ATTRIBUTES = %i[maven_duplicates_allowed maven_duplicate_exception_regex generic_duplicates_allowed - generic_duplicate_exception_regex].freeze + generic_duplicate_exception_regex + maven_package_requests_forwarding + npm_package_requests_forwarding + pypi_package_requests_forwarding + lock_maven_package_requests_forwarding + lock_npm_package_requests_forwarding + lock_pypi_package_requests_forwarding].freeze def execute return ServiceResponse.error(message: 'Access Denied', http_status: 403) unless allowed? diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index b7e6a50fa5c..1aaf7fb769a 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -88,12 +88,14 @@ module Notes return if quick_actions_service.commands_executed_count.to_i == 0 if update_params.present? - if check_for_reviewer_validity(message, update_params) + invalid_message = validate_commands(note, update_params) + + if invalid_message + note.errors.add(:validation, invalid_message) + message = invalid_message + else quick_actions_service.apply_updates(update_params, note) note.commands_changes = update_params - else - message = "Reviewers #{MergeRequest.max_number_of_assignees_or_reviewers_message}" - note.errors.add(:validation, message) end end @@ -114,16 +116,36 @@ module Notes } end - def check_for_reviewer_validity(message, update_params) - return true unless Feature.enabled?(:limit_reviewer_and_assignee_size) + def validate_commands(note, update_params) + if invalid_reviewers?(update_params) + "Reviewers #{note.noteable.class.max_number_of_assignees_or_reviewers_message}" + elsif invalid_assignees?(update_params) + "Assignees #{note.noteable.class.max_number_of_assignees_or_reviewers_message}" + end + end + + def invalid_reviewers?(update_params) + return false unless Feature.enabled?(:limit_reviewer_and_assignee_size) if update_params.key?(:reviewer_ids) possible_reviewers = update_params[:reviewer_ids]&.uniq&.size - return false if possible_reviewers > MergeRequest::MAX_NUMBER_OF_ASSIGNEES_OR_REVIEWERS + possible_reviewers > ::Issuable::MAX_NUMBER_OF_ASSIGNEES_OR_REVIEWERS + else + false end + end + + def invalid_assignees?(update_params) + return false unless Feature.enabled?(:limit_assignees_per_issuable) - true + if update_params.key?(:assignee_ids) + possible_assignees = update_params[:assignee_ids]&.uniq&.size + + possible_assignees > ::Issuable::MAX_NUMBER_OF_ASSIGNEES_OR_REVIEWERS + else + false + end end def track_event(note, user) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 5a92adfd25a..1224cf80b76 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -87,6 +87,13 @@ class NotificationService mailer.access_token_expired_email(user).deliver_later end + # Notify the user when one of their personal access tokens is revoked + def access_token_revoked(user, token_name) + return unless user.can?(:receive_notifications) + + mailer.access_token_revoked_email(user, token_name).deliver_later + end + # Notify the user when at least one of their ssh key has expired today def ssh_key_expired(user, fingerprints) return unless user.can?(:receive_notifications) @@ -109,6 +116,14 @@ class NotificationService mailer.unknown_sign_in_email(user, ip, time).deliver_later end + # Notify a user when a wrong 2FA OTP has been entered to + # try to sign in to their account + def two_factor_otp_attempt_failed(user, ip) + return unless user.can?(:receive_notifications) + + mailer.two_factor_otp_attempt_failed_email(user, ip).deliver_later + end + # Notify a user when a new email address is added to the their account def new_email_address_added(user, email) return unless user.can?(:receive_notifications) diff --git a/app/services/onboarding/progress_service.rb b/app/services/onboarding/progress_service.rb index 66f7f2bc33d..c67669b49ab 100644 --- a/app/services/onboarding/progress_service.rb +++ b/app/services/onboarding/progress_service.rb @@ -12,7 +12,7 @@ module Onboarding def execute(action:) return unless Onboarding::Progress.not_completed?(namespace_id, action) - Namespaces::OnboardingProgressWorker.perform_async(namespace_id, action) + Onboarding::ProgressWorker.perform_async(namespace_id, action) end end diff --git a/app/services/packages/debian/create_package_file_service.rb b/app/services/packages/debian/create_package_file_service.rb index 53275fdc9bb..19e68183ea2 100644 --- a/app/services/packages/debian/create_package_file_service.rb +++ b/app/services/packages/debian/create_package_file_service.rb @@ -5,18 +5,20 @@ module Packages class CreatePackageFileService include ::Packages::FIPS - def initialize(package, params) + def initialize(package:, current_user:, params: {}) @package = package + @current_user = current_user @params = params end def execute raise DisabledError, 'Debian registry is not FIPS compliant' if Gitlab::FIPS.enabled? raise ArgumentError, "Invalid package" unless package.present? + raise ArgumentError, "Invalid user" unless current_user.present? # Debian package file are first uploaded to incoming with empty metadata, # and are moved later by Packages::Debian::ProcessChangesService - package.package_files.create!( + package_file = package.package_files.create!( file: params[:file], size: params[:file]&.size, file_name: params[:file_name], @@ -29,11 +31,17 @@ module Packages fields: nil } ) + + if params[:file_name].end_with? '.changes' + ::Packages::Debian::ProcessChangesWorker.perform_async(package_file.id, current_user.id) + end + + package_file end private - attr_reader :package, :params + attr_reader :package, :current_user, :params end end end diff --git a/app/services/packages/mark_packages_for_destruction_service.rb b/app/services/packages/mark_packages_for_destruction_service.rb new file mode 100644 index 00000000000..023392cf2d9 --- /dev/null +++ b/app/services/packages/mark_packages_for_destruction_service.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +module Packages + class MarkPackagesForDestructionService + include BaseServiceUtility + + BATCH_SIZE = 20 + + UNAUTHORIZED_RESPONSE = ServiceResponse.error( + message: "You don't have the permission to perform this action", + reason: :unauthorized + ).freeze + + ERROR_RESPONSE = ServiceResponse.error( + message: 'Failed to mark the packages as pending destruction' + ).freeze + + SUCCESS_RESPONSE = ServiceResponse.success( + message: 'Packages were successfully marked as pending destruction' + ).freeze + + # Initialize this service with the given packages and user. + # + # * `packages`: must be an ActiveRecord relationship. + # * `current_user`: an User object. Could be nil. + def initialize(packages:, current_user: nil) + @packages = packages + @current_user = current_user + end + + def execute(batch_size: BATCH_SIZE) + no_access = false + min_batch_size = [batch_size, BATCH_SIZE].min + + @packages.each_batch(of: min_batch_size) do |batched_packages| + loaded_packages = batched_packages.including_project_route.to_a + + break no_access = true unless can_destroy_packages?(loaded_packages) + + ::Packages::Package.id_in(loaded_packages.map(&:id)) + .update_all(status: :pending_destruction) + + sync_maven_metadata(loaded_packages) + mark_package_files_for_destruction(loaded_packages) + end + + return UNAUTHORIZED_RESPONSE if no_access + + SUCCESS_RESPONSE + rescue StandardError + ERROR_RESPONSE + end + + private + + def mark_package_files_for_destruction(packages) + ::Packages::MarkPackageFilesForDestructionWorker.bulk_perform_async_with_contexts( + packages, + arguments_proc: -> (package) { package.id }, + context_proc: -> (package) { { project: package.project, user: @current_user } } + ) + end + + def sync_maven_metadata(packages) + maven_packages_with_version = packages.select { |pkg| pkg.maven? && pkg.version? } + ::Packages::Maven::Metadata::SyncWorker.bulk_perform_async_with_contexts( + maven_packages_with_version, + arguments_proc: -> (package) { [@current_user.id, package.project_id, package.name] }, + context_proc: -> (package) { { project: package.project, user: @current_user } } + ) + end + + def can_destroy_packages?(packages) + packages.all? do |package| + can?(@current_user, :destroy_package, package) + end + end + end +end diff --git a/app/services/packages/rpm/parse_package_service.rb b/app/services/packages/rpm/parse_package_service.rb new file mode 100644 index 00000000000..689a161a81a --- /dev/null +++ b/app/services/packages/rpm/parse_package_service.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module Packages + module Rpm + class ParsePackageService + include ::Gitlab::Utils::StrongMemoize + + BUILD_ATTRIBUTES_METHOD_NAMES = %i[changelogs requirements provides].freeze + STATIC_ATTRIBUTES = %i[name version release summary description arch + license sourcerpm group buildhost packager vendor].freeze + + CHANGELOGS_RPM_KEYS = %i[changelogtext changelogtime].freeze + REQUIREMENTS_RPM_KEYS = %i[requirename requireversion requireflags].freeze + PROVIDES_RPM_KEYS = %i[providename provideflags provideversion].freeze + + def initialize(package_file) + @rpm = RPM::File.new(package_file) + end + + def execute + raise ArgumentError, 'Unable to parse package' unless valid_package? + + { + files: rpm.files || [], + epoch: package_tags[:epoch] || '0', + changelogs: build_changelogs, + requirements: build_requirements, + provides: build_provides + }.merge(extract_static_attributes) + end + + private + + attr_reader :rpm + + def valid_package? + rpm.files && package_tags && true + rescue RuntimeError + # if arr-pm throws an error due to an incorrect file format, + # we just want this validation to fail rather than throw an exception + false + end + + def package_tags + strong_memoize(:package_tags) do + rpm.tags + end + end + + def extract_static_attributes + STATIC_ATTRIBUTES.each_with_object({}) do |attribute, hash| + hash[attribute] = package_tags[attribute] + end + end + + # Define methods for building RPM attribute data from parsed package + # Transform + # changelogtime: [123, 234], + # changelogname: ["First", "Second"] + # changelogtext: ["Work1", "Work2"] + # Into + # changelog: [ + # {changelogname: "First", changelogtext: "Work1", changelogtime: 123}, + # {changelogname: "Second", changelogtext: "Work2", changelogtime: 234} + # ] + BUILD_ATTRIBUTES_METHOD_NAMES.each do |resource| + define_method("build_#{resource}") do + resource_keys = self.class.const_get("#{resource.upcase}_RPM_KEYS", false).dup + return [] if resource_keys.any? { package_tags[_1].blank? } + + first_attributes = package_tags[resource_keys.first] + zipped_data = first_attributes.zip(*resource_keys[1..].map { package_tags[_1] }) + build_hashes(resource_keys, zipped_data) + end + end + + def build_hashes(resource_keys, zipped_data) + zipped_data.map do |data| + resource_keys.zip(data).to_h + end + end + end + end +end diff --git a/app/services/packages/rpm/repository_metadata/base_builder.rb b/app/services/packages/rpm/repository_metadata/base_builder.rb index 9d76336d764..2c0a11457ec 100644 --- a/app/services/packages/rpm/repository_metadata/base_builder.rb +++ b/app/services/packages/rpm/repository_metadata/base_builder.rb @@ -3,17 +3,43 @@ module Packages module Rpm module RepositoryMetadata class BaseBuilder + def initialize(xml: nil, data: {}) + @xml = Nokogiri::XML(xml) if xml.present? + @data = data + end + def execute - build_empty_structure + return build_empty_structure if xml.blank? + + update_xml_document + update_package_count + xml.to_xml end private + attr_reader :xml, :data + def build_empty_structure Nokogiri::XML::Builder.new(encoding: 'UTF-8') do |xml| - xml.public_send(self.class::ROOT_TAG, self.class::ROOT_ATTRIBUTES) # rubocop:disable GitlabSecurity/PublicSend + xml.method_missing(self.class::ROOT_TAG, self.class::ROOT_ATTRIBUTES) end.to_xml end + + def update_xml_document + # Add to the root xml element a new package metadata node + xml.at(self.class::ROOT_TAG).add_child(build_new_node) + end + + def update_package_count + packages_count = xml.css("//#{self.class::ROOT_TAG}/package").count + + xml.at(self.class::ROOT_TAG).attributes["packages"].value = packages_count.to_s + end + + def build_new_node + raise NotImplementedError, "#{self.class} should implement #{__method__}" + end end end end diff --git a/app/services/packages/rpm/repository_metadata/build_primary_xml.rb b/app/services/packages/rpm/repository_metadata/build_primary_xml.rb index affb41677c2..580bf844a0c 100644 --- a/app/services/packages/rpm/repository_metadata/build_primary_xml.rb +++ b/app/services/packages/rpm/repository_metadata/build_primary_xml.rb @@ -9,6 +9,79 @@ module Packages 'xmlns:rpm': 'http://linux.duke.edu/metadata/rpm', packages: '0' }.freeze + + # Nodes that have only text without attributes + REQUIRED_BASE_ATTRIBUTES = %i[name arch summary description].freeze + NOT_REQUIRED_BASE_ATTRIBUTES = %i[url packager].freeze + FORMAT_NODE_BASE_ATTRIBUTES = %i[license vendor group buildhost sourcerpm].freeze + + private + + def build_new_node + builder = Nokogiri::XML::Builder.new do |xml| + xml.package(type: :rpm, 'xmlns:rpm': 'http://linux.duke.edu/metadata/rpm') do + build_required_base_attributes(xml) + build_not_required_base_attributes(xml) + xml.version epoch: data[:epoch], ver: data[:version], rel: data[:release] + xml.checksum data[:checksum], type: 'sha256', pkgid: 'YES' + xml.size package: data[:packagesize], installed: data[:installedsize], archive: data[:archivesize] + xml.time file: data[:filetime], build: data[:buildtime] + xml.location href: data[:location] if data[:location].present? + build_format_node(xml) + end + end + + Nokogiri::XML(builder.to_xml).at('package') + end + + def build_required_base_attributes(xml) + REQUIRED_BASE_ATTRIBUTES.each do |attribute| + xml.method_missing(attribute, data[attribute]) + end + end + + def build_not_required_base_attributes(xml) + NOT_REQUIRED_BASE_ATTRIBUTES.each do |attribute| + xml.method_missing(attribute, data[attribute]) if data[attribute].present? + end + end + + def build_format_node(xml) + xml.format do + build_base_format_attributes(xml) + build_provides_node(xml) + build_requires_node(xml) + end + end + + def build_base_format_attributes(xml) + FORMAT_NODE_BASE_ATTRIBUTES.each do |attribute| + xml[:rpm].method_missing(attribute, data[attribute]) if data[attribute].present? + end + end + + def build_requires_node(xml) + xml[:rpm].requires do + data[:requirements].each do |requires| + xml[:rpm].entry( + name: requires[:requirename], + flags: requires[:requireflags], + ver: requires[:requireversion] + ) + end + end + end + + def build_provides_node(xml) + xml[:rpm].provides do + data[:provides].each do |provides| + xml[:rpm].entry( + name: provides[:providename], + flags: provides[:provideflags], + ver: provides[:provideversion]) + end + end + end end end end diff --git a/app/services/packages/rpm/repository_metadata/build_repomd_xml.rb b/app/services/packages/rpm/repository_metadata/build_repomd_xml.rb index c6cfd77815d..84614196254 100644 --- a/app/services/packages/rpm/repository_metadata/build_repomd_xml.rb +++ b/app/services/packages/rpm/repository_metadata/build_repomd_xml.rb @@ -9,6 +9,7 @@ module Packages xmlns: 'http://linux.duke.edu/metadata/repo', 'xmlns:rpm': 'http://linux.duke.edu/metadata/rpm' }.freeze + ALLOWED_DATA_VALUE_KEYS = %i[checksum open-checksum location timestamp size open-size].freeze # Expected `data` structure # @@ -48,9 +49,9 @@ module Packages end def build_file_info(info, xml) - info.each do |key, attributes| + info.slice(*ALLOWED_DATA_VALUE_KEYS).each do |key, attributes| value = attributes.delete(:value) - xml.public_send(key, value, attributes) # rubocop:disable GitlabSecurity/PublicSend + xml.method_missing(key, value, attributes) end end end diff --git a/app/services/pages_domains/create_acme_order_service.rb b/app/services/pages_domains/create_acme_order_service.rb index e289a78091b..c600f497fa5 100644 --- a/app/services/pages_domains/create_acme_order_service.rb +++ b/app/services/pages_domains/create_acme_order_service.rb @@ -2,9 +2,6 @@ module PagesDomains class CreateAcmeOrderService - # elliptic curve algorithm to generate the private key - ECDSA_CURVE = "prime256v1" - attr_reader :pages_domain def initialize(pages_domain) @@ -17,12 +14,7 @@ module PagesDomains challenge = order.new_challenge - private_key = if Feature.enabled?(:pages_lets_encrypt_ecdsa, pages_domain.project) - OpenSSL::PKey::EC.generate(ECDSA_CURVE) - else - OpenSSL::PKey::RSA.new(4096) - end - + private_key = OpenSSL::PKey::RSA.new(4096) saved_order = pages_domain.acme_orders.create!( url: order.url, expires_at: order.expires, diff --git a/app/services/pages_domains/create_service.rb b/app/services/pages_domains/create_service.rb new file mode 100644 index 00000000000..1f771ca3a05 --- /dev/null +++ b/app/services/pages_domains/create_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module PagesDomains + class CreateService < BaseService + def execute + return unless authorized? + + domain = project.pages_domains.create(params) + + publish_event(domain) if domain.persisted? + + domain + end + + private + + def authorized? + current_user.can?(:update_pages, project) + end + + def publish_event(domain) + event = PagesDomainCreatedEvent.new( + data: { + project_id: project.id, + namespace_id: project.namespace_id, + root_namespace_id: project.root_namespace.id, + domain: domain.domain + } + ) + + Gitlab::EventStore.publish(event) + end + end +end diff --git a/app/services/pages_domains/delete_service.rb b/app/services/pages_domains/delete_service.rb new file mode 100644 index 00000000000..af69e1845a9 --- /dev/null +++ b/app/services/pages_domains/delete_service.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module PagesDomains + class DeleteService < BaseService + def execute(domain) + return unless authorized? + + domain.destroy + + publish_event(domain) + end + + private + + def authorized? + current_user.can?(:update_pages, project) + end + + def publish_event(domain) + event = PagesDomainDeletedEvent.new( + data: { + project_id: project.id, + namespace_id: project.namespace_id, + root_namespace_id: project.root_namespace.id, + domain: domain.domain + } + ) + + Gitlab::EventStore.publish(event) + end + end +end diff --git a/app/services/pages_domains/update_service.rb b/app/services/pages_domains/update_service.rb new file mode 100644 index 00000000000..b038aaa95b6 --- /dev/null +++ b/app/services/pages_domains/update_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module PagesDomains + class UpdateService < BaseService + def execute(domain) + return unless authorized? + + return false unless domain.update(params) + + publish_event(domain) + + true + end + + private + + def authorized? + current_user.can?(:update_pages, project) + end + + def publish_event(domain) + event = PagesDomainUpdatedEvent.new( + data: { + project_id: project.id, + namespace_id: project.namespace_id, + root_namespace_id: project.root_namespace.id, + domain: domain.domain + } + ) + + Gitlab::EventStore.publish(event) + end + end +end diff --git a/app/services/personal_access_tokens/revoke_service.rb b/app/services/personal_access_tokens/revoke_service.rb index 0275d03bcc9..732da75da3a 100644 --- a/app/services/personal_access_tokens/revoke_service.rb +++ b/app/services/personal_access_tokens/revoke_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module PersonalAccessTokens - class RevokeService + class RevokeService < BaseService attr_reader :token, :current_user, :group def initialize(current_user = nil, token: nil, group: nil ) @@ -15,6 +15,7 @@ module PersonalAccessTokens if token.revoke! log_event + notification_service.access_token_revoked(token.user, token.name) ServiceResponse.success(message: success_message) else ServiceResponse.error(message: error_message) diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index e6b1b33a82a..ae5aae87a77 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -24,7 +24,7 @@ module Projects end def commands(noteable, type) - return [] unless noteable + return [] unless noteable && current_user QuickActions::InterpretService.new(project, current_user).available_commands(noteable) end @@ -33,9 +33,21 @@ 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]) + def contacts(target) + available_contacts = Crm::ContactsFinder.new(current_user, group: project.group).execute + .select([:id, :email, :first_name, :last_name, :state]) + + contact_hashes = available_contacts.as_json + + return contact_hashes unless target.is_a?(Issue) + + ids = target.customer_relations_contacts.ids # rubocop:disable CodeReuse/ActiveRecord + + contact_hashes.each do |hash| + hash[:set] = ids.include?(hash['id']) + end + + contact_hashes end def labels_as_hash(target) diff --git a/app/services/projects/blame_service.rb b/app/services/projects/blame_service.rb index 57b913b04e6..58e146e5a32 100644 --- a/app/services/projects/blame_service.rb +++ b/app/services/projects/blame_service.rb @@ -27,6 +27,10 @@ module Projects .page(page) end + def per_page + PER_PAGE + end + private attr_reader :blob, :commit, :pagination_enabled @@ -48,10 +52,6 @@ module Projects page end - def per_page - PER_PAGE - end - def pagination_state(params) return false if Gitlab::Utils.to_boolean(params[:no_pagination], default: false) diff --git a/app/services/projects/container_repository/cleanup_tags_base_service.rb b/app/services/projects/container_repository/cleanup_tags_base_service.rb index 8ea4ae4830a..5393c2c080d 100644 --- a/app/services/projects/container_repository/cleanup_tags_base_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_base_service.rb @@ -60,23 +60,6 @@ module Projects service.execute(container_repository) end - def can_destroy? - return true if container_expiration_policy - - can?(current_user, :destroy_container_image, project) - end - - def valid_regex? - %w[name_regex_delete name_regex name_regex_keep].each do |param_name| - regex = params[param_name] - ::Gitlab::UntrustedRegexp.new(regex) unless regex.blank? - end - true - rescue RegexpError => e - ::Gitlab::ErrorTracking.log_exception(e, project_id: project.id) - false - end - def older_than params['older_than'] end diff --git a/app/services/projects/container_repository/cleanup_tags_service.rb b/app/services/projects/container_repository/cleanup_tags_service.rb index 285c3e252ef..cf2eb81e5f3 100644 --- a/app/services/projects/container_repository/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_service.rb @@ -2,101 +2,57 @@ module Projects module ContainerRepository - class CleanupTagsService < CleanupTagsBaseService - def initialize(container_repository:, current_user: nil, params: {}) - super - - @params = params.dup - @counts = { cached_tags_count: 0 } - end - + class CleanupTagsService < BaseContainerRepositoryService def execute return error('access denied') unless can_destroy? return error('invalid regex') unless valid_regex? - tags = container_repository.tags - @counts[:original_size] = tags.size - - filter_out_latest!(tags) - filter_by_name!(tags) - - tags = truncate(tags) - populate_from_cache(tags) - - tags = filter_keep_n(tags) - tags = filter_by_older_than(tags) - - @counts[:before_delete_size] = tags.size - - delete_tags(tags).merge(@counts).tap do |result| - result[:deleted_size] = result[:deleted]&.size - - result[:status] = :error if @counts[:before_truncate_size] != @counts[:after_truncate_size] - end + cleanup_tags_service_class.new(container_repository: container_repository, current_user: current_user, params: params) + .execute end private - def filter_keep_n(tags) - tags, tags_to_keep = partition_by_keep_n(tags) - - cache_tags(tags_to_keep) - - tags - end - - def filter_by_older_than(tags) - tags, tags_to_keep = partition_by_older_than(tags) - - cache_tags(tags_to_keep) - - tags + def cleanup_tags_service_class + log_data = { + container_repository_id: container_repository.id, + container_repository_path: container_repository.path, + project_id: project.id + } + + if use_gitlab_service? + log_info(log_data.merge(gitlab_cleanup_tags_service: true)) + ::Projects::ContainerRepository::Gitlab::CleanupTagsService + else + log_info(log_data.merge(third_party_cleanup_tags_service: true)) + ::Projects::ContainerRepository::ThirdParty::CleanupTagsService + end end - def pushed_at(tag) - tag.created_at + def use_gitlab_service? + container_repository.migrated? && + container_repository.gitlab_api_client.supports_gitlab_api? end - def truncate(tags) - @counts[:before_truncate_size] = tags.size - @counts[:after_truncate_size] = tags.size - - return tags if max_list_size == 0 - - # truncate the list to make sure that after the #filter_keep_n - # execution, the resulting list will be max_list_size - truncated_size = max_list_size + keep_n_as_integer - - return tags if tags.size <= truncated_size + def can_destroy? + return true if container_expiration_policy - tags = tags.sample(truncated_size) - @counts[:after_truncate_size] = tags.size - tags + can?(current_user, :destroy_container_image, project) end - def populate_from_cache(tags) - @counts[:cached_tags_count] = cache.populate(tags) if caching_enabled? - end - - def cache_tags(tags) - cache.insert(tags, older_than_in_seconds) if caching_enabled? - end - - def cache - strong_memoize(:cache) do - ::Gitlab::ContainerRepository::Tags::Cache.new(container_repository) + def valid_regex? + %w[name_regex_delete name_regex name_regex_keep].each do |param_name| + regex = params[param_name] + ::Gitlab::UntrustedRegexp.new(regex) unless regex.blank? end + true + rescue RegexpError => e + ::Gitlab::ErrorTracking.log_exception(e, project_id: project.id) + false end - def caching_enabled? - result = ::Gitlab::CurrentSettings.current_application_settings.container_registry_expiration_policies_caching && - container_expiration_policy && - older_than.present? - !!result - end - - def max_list_size - ::Gitlab::CurrentSettings.current_application_settings.container_registry_cleanup_tags_service_max_list_size.to_i + def container_expiration_policy + params['container_expiration_policy'] end end end diff --git a/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb b/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb index 81bb94c867a..e947e9575e2 100644 --- a/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb @@ -14,9 +14,6 @@ module Projects end def execute - return error('access denied') unless can_destroy? - return error('invalid regex') unless valid_regex? - with_timeout do |start_time, result| container_repository.each_tags_page(page_size: TAGS_PAGE_SIZE) do |tags| execute_for_tags(tags, result) diff --git a/app/services/projects/container_repository/third_party/cleanup_tags_service.rb b/app/services/projects/container_repository/third_party/cleanup_tags_service.rb new file mode 100644 index 00000000000..c6335629b52 --- /dev/null +++ b/app/services/projects/container_repository/third_party/cleanup_tags_service.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +module Projects + module ContainerRepository + module ThirdParty + class CleanupTagsService < CleanupTagsBaseService + def initialize(container_repository:, current_user: nil, params: {}) + super + + @params = params.dup + @counts = { cached_tags_count: 0 } + end + + def execute + tags = container_repository.tags + @counts[:original_size] = tags.size + + filter_out_latest!(tags) + filter_by_name!(tags) + + tags = truncate(tags) + populate_from_cache(tags) + + tags = filter_keep_n(tags) + tags = filter_by_older_than(tags) + + @counts[:before_delete_size] = tags.size + + delete_tags(tags).merge(@counts).tap do |result| + result[:deleted_size] = result[:deleted]&.size + + result[:status] = :error if @counts[:before_truncate_size] != @counts[:after_truncate_size] + end + end + + private + + def filter_keep_n(tags) + tags, tags_to_keep = partition_by_keep_n(tags) + + cache_tags(tags_to_keep) + + tags + end + + def filter_by_older_than(tags) + tags, tags_to_keep = partition_by_older_than(tags) + + cache_tags(tags_to_keep) + + tags + end + + def pushed_at(tag) + tag.created_at + end + + def truncate(tags) + @counts[:before_truncate_size] = tags.size + @counts[:after_truncate_size] = tags.size + + return tags if max_list_size == 0 + + # truncate the list to make sure that after the #filter_keep_n + # execution, the resulting list will be max_list_size + truncated_size = max_list_size + keep_n_as_integer + + return tags if tags.size <= truncated_size + + tags = tags.sample(truncated_size) + @counts[:after_truncate_size] = tags.size + tags + end + + def populate_from_cache(tags) + @counts[:cached_tags_count] = cache.populate(tags) if caching_enabled? + end + + def cache_tags(tags) + cache.insert(tags, older_than_in_seconds) if caching_enabled? + end + + def cache + strong_memoize(:cache) do + ::Gitlab::ContainerRepository::Tags::Cache.new(container_repository) + end + end + + def caching_enabled? + result = current_application_settings.container_registry_expiration_policies_caching && + container_expiration_policy && + older_than.present? + !!result + end + + def max_list_size + current_application_settings.container_registry_cleanup_tags_service_max_list_size.to_i + end + + def current_application_settings + ::Gitlab::CurrentSettings.current_application_settings + end + end + end + end +end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index f1525ed9763..4e883f682fb 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -134,7 +134,7 @@ module Projects destroy_ci_records! destroy_mr_diff_relations! - destroy_merge_request_diffs! if ::Feature.enabled?(:extract_mr_diff_deletions) + destroy_merge_request_diffs! # Rails attempts to load all related records into memory before # destroying: https://github.com/rails/rails/issues/22510 diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 4979af6dfe1..de7ede4eabf 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -64,7 +64,11 @@ module Projects def add_repository_to_project if project.external_import? && !unknown_url? begin - Gitlab::UrlBlocker.validate!(project.import_url, ports: Project::VALID_IMPORT_PORTS) + Gitlab::UrlBlocker.validate!( + project.import_url, + schemes: Project::VALID_IMPORT_PROTOCOLS, + ports: Project::VALID_IMPORT_PORTS + ) rescue Gitlab::UrlBlocker::BlockedUrlError => e raise e, s_("ImportProjects|Blocked import URL: %{message}") % { message: e.message } end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index d757b0700b9..f9a2c825608 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -122,7 +122,7 @@ module Projects update_pending_builds if runners_settings_toggled? - publish_event + publish_events end def after_rename_service(project) @@ -212,7 +212,13 @@ module Projects end end - def publish_event + def publish_events + publish_project_archived_event + publish_project_attributed_changed_event + publish_project_features_changed_event + end + + def publish_project_archived_event return unless project.archived_previously_changed? event = Projects::ProjectArchivedEvent.new(data: { @@ -223,6 +229,36 @@ module Projects Gitlab::EventStore.publish(event) end + + def publish_project_attributed_changed_event + changes = @project.previous_changes + + return if changes.blank? + + event = Projects::ProjectAttributesChangedEvent.new(data: { + project_id: @project.id, + namespace_id: @project.namespace_id, + root_namespace_id: @project.root_namespace.id, + attributes: changes.keys + }) + + Gitlab::EventStore.publish(event) + end + + def publish_project_features_changed_event + changes = @project.project_feature.previous_changes + + return if changes.blank? + + event = Projects::ProjectFeaturesChangedEvent.new(data: { + project_id: @project.id, + namespace_id: @project.namespace_id, + root_namespace_id: @project.root_namespace.id, + features: changes.keys + }) + + Gitlab::EventStore.publish(event) + end end end diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb index b7df201824a..01dd6323d94 100644 --- a/app/services/releases/create_service.rb +++ b/app/services/releases/create_service.rb @@ -3,10 +3,10 @@ module Releases class CreateService < Releases::BaseService def execute - return error('Access Denied', 403) unless allowed? - return error('You are not allowed to create this tag as it is protected.', 403) unless can_create_tag? - return error('Release already exists', 409) if release - return error("Milestone(s) not found: #{inexistent_milestones.join(', ')}", 400) if inexistent_milestones.any? + return error(_('Access Denied'), 403) unless allowed? + return error(_('You are not allowed to create this tag as it is protected.'), 403) unless can_create_tag? + return error(_('Release already exists'), 409) if release + return error(format(_("Milestone(s) not found: %{milestones}"), milestones: inexistent_milestones.join(', ')), 400) if inexistent_milestones.any? # rubocop:disable Layout/LineLength # should be found before the creation of new tag # because tag creation can spawn new pipeline diff --git a/app/services/releases/destroy_service.rb b/app/services/releases/destroy_service.rb index 8abf9308689..ff2b3a7bd18 100644 --- a/app/services/releases/destroy_service.rb +++ b/app/services/releases/destroy_service.rb @@ -3,8 +3,8 @@ module Releases class DestroyService < Releases::BaseService def execute - return error('Release does not exist', 404) unless release - return error('Access Denied', 403) unless allowed? + return error(_('Release does not exist'), 404) unless release + return error(_('Access Denied'), 403) unless allowed? if release.destroy success(tag: existing_tag, release: release) diff --git a/app/services/releases/update_service.rb b/app/services/releases/update_service.rb index 2e0a2f8488a..b9b2aba9805 100644 --- a/app/services/releases/update_service.rb +++ b/app/services/releases/update_service.rb @@ -31,11 +31,11 @@ module Releases private def validate - return error('Tag does not exist', 404) unless existing_tag - return error('Release does not exist', 404) unless release - return error('Access Denied', 403) unless allowed? - return error('params is empty', 400) if empty_params? - return error("Milestone(s) not found: #{inexistent_milestones.join(', ')}", 400) if inexistent_milestones.any? + return error(_('Tag does not exist'), 404) unless existing_tag + return error(_('Release does not exist'), 404) unless release + return error(_('Access Denied'), 403) unless allowed? + return error(_('params is empty'), 400) if empty_params? + return error(format(_("Milestone(s) not found: %{milestones}"), milestones: inexistent_milestones.join(', ')), 400) if inexistent_milestones.any? # rubocop:disable Layout/LineLength end def allowed? diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index eed03ba22fe..b8a210c0a95 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -13,7 +13,6 @@ module ResourceAccessTokens return error("User does not have permission to create #{resource_type} access token") unless has_permission_to_create? access_level = params[:access_level] || Gitlab::Access::MAINTAINER - return error("Could not provision owner access to project access token") if do_not_allow_owner_access_level_for_project_bot?(access_level) user = create_user @@ -48,9 +47,9 @@ module ResourceAccessTokens end def create_user - # Even project maintainers can create project access tokens, which in turn + # Even project maintainers/owners can create project access tokens, which in turn # creates a bot user, and so it becomes necessary to have `skip_authorization: true` - # since someone like a project maintainer does not inherently have the ability + # since someone like a project maintainer/owner does not inherently have the ability # to create a new user in the system. ::Users::AuthorizedCreateService.new(current_user, default_user_params).execute @@ -108,7 +107,7 @@ module ResourceAccessTokens end def create_membership(resource, user, access_level) - resource.add_member(user, access_level, expires_at: params[:expires_at]) + resource.add_member(user, access_level, current_user: current_user, expires_at: params[:expires_at]) end def log_event(token) @@ -122,12 +121,6 @@ module ResourceAccessTokens def success(access_token) ServiceResponse.success(payload: { access_token: access_token }) end - - def do_not_allow_owner_access_level_for_project_bot?(access_level) - resource.is_a?(Project) && - access_level == Gitlab::Access::OWNER && - !current_user.can?(:manage_owners, resource) - end end end diff --git a/app/services/search_service.rb b/app/services/search_service.rb index cea7fc5769e..f38522b9764 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -102,6 +102,16 @@ class SearchService end end + def show_elasticsearch_tabs? + # overridden in EE + false + end + + def show_epics? + # overridden in EE + false + end + private def page diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index ddb20a835e1..0fa1bb96b13 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -106,6 +106,8 @@ module Users def build_user_params_for_non_admin @user_params = params.slice(*signup_params) + # if skip_confirmation is set to `true`, devise will set confirmed_at + # see: https://github.com/heartcombo/devise/blob/8593801130f2df94a50863b5db535c272b00efe1/lib/devise/models/confirmable.rb#L156 @user_params[:skip_confirmation] = skip_user_confirmation_email_from_setting if assign_skip_confirmation_from_settings? @user_params[:name] = fallback_name if use_fallback_name? end diff --git a/app/services/users/dismiss_namespace_callout_service.rb b/app/services/users/dismiss_namespace_callout_service.rb deleted file mode 100644 index 51261a93e20..00000000000 --- a/app/services/users/dismiss_namespace_callout_service.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module Users - class DismissNamespaceCalloutService < DismissCalloutService - private - - def callout - current_user.find_or_initialize_namespace_callout(params[:feature_name], params[:namespace_id]) - end - end -end diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index fe61335f3ed..b1ffd006795 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -62,12 +62,12 @@ module Users # Updates the list of authorizations for the current user. # - # remove - The IDs of the authorization rows to remove. + # remove - The project IDs of the authorization rows to remove. # add - Rows to insert in the form `[{ user_id: user_id, project_id: project_id, access_level: access_level}, ...]` def update_authorizations(remove = [], add = []) log_refresh_details(remove, add) - user.remove_project_authorizations(remove) if remove.any? + ProjectAuthorization.delete_all_in_batches_for_user(user: user, project_ids: remove) if remove.any? ProjectAuthorization.insert_all_in_batches(add) if add.any? # Since we batch insert authorization rows, Rails' associations may get diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index cd2c7402713..e5e5e375198 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -194,7 +194,8 @@ class WebHookService headers = { 'Content-Type' => 'application/json', 'User-Agent' => "GitLab/#{Gitlab::VERSION}", - Gitlab::WebHooks::GITLAB_EVENT_HEADER => self.class.hook_to_event(hook_name) + Gitlab::WebHooks::GITLAB_EVENT_HEADER => self.class.hook_to_event(hook_name), + Gitlab::WebHooks::GITLAB_INSTANCE_HEADER => Gitlab.config.gitlab.base_url } headers['X-Gitlab-Token'] = Gitlab::Utils.remove_line_breaks(hook.token) if hook.token.present? diff --git a/app/services/web_hooks/log_execution_service.rb b/app/services/web_hooks/log_execution_service.rb index 5be8aee3ae8..1a40c877bda 100644 --- a/app/services/web_hooks/log_execution_service.rb +++ b/app/services/web_hooks/log_execution_service.rb @@ -17,7 +17,7 @@ module WebHooks end def execute - update_hook_failure_state + update_hook_failure_state if WebHook.web_hooks_disable_failed?(hook) log_execution end diff --git a/app/services/work_items/create_service.rb b/app/services/work_items/create_service.rb index c2ceb701a2f..ebda043e873 100644 --- a/app/services/work_items/create_service.rb +++ b/app/services/work_items/create_service.rb @@ -2,7 +2,6 @@ module WorkItems class CreateService < Issues::CreateService - include ::Services::ReturnServiceResponses include WidgetableService def initialize(project:, current_user: nil, params: {}, spam_params:, widget_params: {}) @@ -17,11 +16,10 @@ module WorkItems end def execute - unless @current_user.can?(:create_work_item, @project) - return error(_('Operation not allowed'), :forbidden) - end + result = super + return result if result.error? - work_item = super + work_item = result[:issue] if work_item.valid? success(payload(work_item)) @@ -43,6 +41,10 @@ module WorkItems private + def authorization_action + :create_work_item + end + def payload(work_item) { work_item: work_item } end diff --git a/app/services/work_items/update_service.rb b/app/services/work_items/update_service.rb index 2deb8c82741..1351445f6f3 100644 --- a/app/services/work_items/update_service.rb +++ b/app/services/work_items/update_service.rb @@ -26,6 +26,17 @@ module WorkItems private + def prepare_update_params(work_item) + execute_widgets( + work_item: work_item, + callback: :prepare_update_params, + widget_params: @widget_params, + service_params: params + ) + + super + end + def before_update(work_item, skip_spam_check: false) execute_widgets(work_item: work_item, callback: :before_update_callback, widget_params: @widget_params) @@ -38,7 +49,7 @@ module WorkItems super end - def after_update(work_item) + def after_update(work_item, old_associations) super GraphqlTriggers.issuable_title_updated(work_item) if work_item.previous_changes.key?(:title) @@ -47,5 +58,13 @@ module WorkItems def payload(work_item) { work_item: work_item } end + + def handle_label_changes(issuable, old_labels) + return false unless super + + Gitlab::UsageDataCounters::WorkItemActivityUniqueCounter.track_work_item_labels_changed_action( + author: current_user + ) + end end end diff --git a/app/services/work_items/widgets/base_service.rb b/app/services/work_items/widgets/base_service.rb index 37ed2bf4b05..1ff03a09f9f 100644 --- a/app/services/work_items/widgets/base_service.rb +++ b/app/services/work_items/widgets/base_service.rb @@ -5,12 +5,13 @@ module WorkItems class BaseService < ::BaseService WidgetError = Class.new(StandardError) - attr_reader :widget, :work_item, :current_user + attr_reader :widget, :work_item, :current_user, :service_params - def initialize(widget:, current_user:) + def initialize(widget:, current_user:, service_params: {}) @widget = widget @work_item = widget.work_item @current_user = current_user + @service_params = service_params end private diff --git a/app/services/work_items/widgets/labels_service/update_service.rb b/app/services/work_items/widgets/labels_service/update_service.rb new file mode 100644 index 00000000000..f00ea5c95ca --- /dev/null +++ b/app/services/work_items/widgets/labels_service/update_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module WorkItems + module Widgets + module LabelsService + class UpdateService < WorkItems::Widgets::BaseService + def prepare_update_params(params: {}) + return if params.blank? + + service_params.merge!(params.slice(:add_label_ids, :remove_label_ids)) + end + end + end + end +end |