diff options
77 files changed, 990 insertions, 144 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index f3a0458d512..154c5f58c35 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -871f4415ed24c64e6a2746a4502f0bea38d0c440 +7cd8ef212eccee3ffd388f1b88ccdeca15971435 diff --git a/app/controllers/projects/google_cloud/service_accounts_controller.rb b/app/controllers/projects/google_cloud/service_accounts_controller.rb index 31ef1463056..b5f2b658235 100644 --- a/app/controllers/projects/google_cloud/service_accounts_controller.rb +++ b/app/controllers/projects/google_cloud/service_accounts_controller.rb @@ -24,24 +24,15 @@ class Projects::GoogleCloud::ServiceAccountsController < Projects::GoogleCloud:: end def create - google_api_client = GoogleApi::CloudPlatform::Client.new(token_in_session, nil) - service_accounts_service = GoogleCloud::ServiceAccountsService.new(project) - gcp_project = params[:gcp_project] - environment = params[:environment] - generated_name = "GitLab :: #{@project.name} :: #{environment}" - generated_desc = "GitLab generated service account for project '#{@project.name}' and environment '#{environment}'" - - service_account = google_api_client.create_service_account(gcp_project, generated_name, generated_desc) - service_account_key = google_api_client.create_service_account_key(gcp_project, service_account.unique_id) - - service_accounts_service.add_for_project( - environment, - service_account.project_id, - service_account.to_json, - service_account_key.to_json - ) + response = GoogleCloud::CreateServiceAccountsService.new( + project, + current_user, + google_oauth2_token: token_in_session, + gcp_project_id: params[:gcp_project], + environment_name: params[:environment] + ).execute - redirect_to project_google_cloud_index_path(project), notice: _('Service account generated successfully') + redirect_to project_google_cloud_index_path(project), notice: response.message rescue Google::Apis::ClientError, Google::Apis::ServerError, Google::Apis::AuthorizationError => error handle_gcp_error(error, project) end diff --git a/app/controllers/projects/packages/infrastructure_registry_controller.rb b/app/controllers/projects/packages/infrastructure_registry_controller.rb index 4af2894590b..c02a0a56e03 100644 --- a/app/controllers/projects/packages/infrastructure_registry_controller.rb +++ b/app/controllers/projects/packages/infrastructure_registry_controller.rb @@ -9,7 +9,7 @@ module Projects def show @package = project.packages.find(params[:id]) - @package_files = if Feature.enabled?(:packages_installable_package_files) + @package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) @package.installable_package_files.recent else @package.package_files.recent diff --git a/app/finders/packages/package_file_finder.rb b/app/finders/packages/package_file_finder.rb index a47a5af7325..55dc4be2001 100644 --- a/app/finders/packages/package_file_finder.rb +++ b/app/finders/packages/package_file_finder.rb @@ -19,7 +19,7 @@ class Packages::PackageFileFinder private def package_files - files = if Feature.enabled?(:packages_installable_package_files) + files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) package.installable_package_files else package.package_files diff --git a/app/graphql/types/packages/package_details_type.rb b/app/graphql/types/packages/package_details_type.rb index 07d8cf93b4e..1d2cf9649d8 100644 --- a/app/graphql/types/packages/package_details_type.rb +++ b/app/graphql/types/packages/package_details_type.rb @@ -37,7 +37,7 @@ module Types end def package_files - if Feature.enabled?(:packages_installable_package_files) + if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) object.installable_package_files else object.package_files diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index ef5e72342fb..e6aa3d525a9 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -365,6 +365,10 @@ class ApplicationSetting < ApplicationRecord allow_nil: false, numericality: { only_integer: true, greater_than_or_equal_to: 0 } + validates :packages_cleanup_package_file_worker_capacity, + allow_nil: false, + numericality: { only_integer: true, greater_than_or_equal_to: 0 } + validates :invisible_captcha_enabled, inclusion: { in: [true, false], message: _('must be a boolean value') } diff --git a/app/models/concerns/packages/debian/distribution.rb b/app/models/concerns/packages/debian/distribution.rb index 3334cd69ed3..2d46889ce6a 100644 --- a/app/models/concerns/packages/debian/distribution.rb +++ b/app/models/concerns/packages/debian/distribution.rb @@ -97,7 +97,7 @@ module Packages end def package_files - if Feature.enabled?(:packages_installable_package_files) + if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) ::Packages::PackageFile.installable .for_package_ids(packages.select(:id)) else diff --git a/app/models/concerns/packages/destructible.rb b/app/models/concerns/packages/destructible.rb new file mode 100644 index 00000000000..a3b7d8580c1 --- /dev/null +++ b/app/models/concerns/packages/destructible.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Packages + module Destructible + extend ActiveSupport::Concern + + class_methods do + def next_pending_destruction(order_by: nil) + set = pending_destruction.limit(1).lock('FOR UPDATE SKIP LOCKED') + set = set.order(order_by) if order_by + set.take + end + end + end +end diff --git a/app/models/concerns/ttl_expirable.rb b/app/models/concerns/ttl_expirable.rb index 6d89521255c..1c2147beedd 100644 --- a/app/models/concerns/ttl_expirable.rb +++ b/app/models/concerns/ttl_expirable.rb @@ -7,16 +7,10 @@ module TtlExpirable validates :status, presence: true default_value_for :read_at, Time.zone.now - enum status: { default: 0, expired: 1, processing: 2, error: 3 } + enum status: { default: 0, pending_destruction: 1, processing: 2, error: 3 } scope :read_before, ->(number_of_days) { where("read_at <= ?", Time.zone.now - number_of_days.days) } scope :active, -> { where(status: :default) } - - scope :lock_next_by, ->(sort) do - order(sort) - .limit(1) - .lock('FOR UPDATE SKIP LOCKED') - end end def read! diff --git a/app/models/dependency_proxy/blob.rb b/app/models/dependency_proxy/blob.rb index bd5c022e692..e4018ab4770 100644 --- a/app/models/dependency_proxy/blob.rb +++ b/app/models/dependency_proxy/blob.rb @@ -3,6 +3,7 @@ class DependencyProxy::Blob < ApplicationRecord include FileStoreMounter include TtlExpirable + include Packages::Destructible include EachBatch belongs_to :group diff --git a/app/models/dependency_proxy/manifest.rb b/app/models/dependency_proxy/manifest.rb index 64f484942ef..fe887c99e81 100644 --- a/app/models/dependency_proxy/manifest.rb +++ b/app/models/dependency_proxy/manifest.rb @@ -3,6 +3,7 @@ class DependencyProxy::Manifest < ApplicationRecord include FileStoreMounter include TtlExpirable + include Packages::Destructible include EachBatch belongs_to :group diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index ebb9774b875..52dd0aba43b 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -133,7 +133,7 @@ class Packages::Package < ApplicationRecord scope :without_nuget_temporary_name, -> { where.not(name: Packages::Nuget::TEMPORARY_PACKAGE_NAME) } scope :has_version, -> { where.not(version: nil) } - scope :preload_files, -> { Feature.enabled?(:packages_installable_package_files) ? preload(:installable_package_files) : preload(:package_files) } + scope :preload_files, -> { Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) ? preload(:installable_package_files) : preload(:package_files) } scope :preload_pipelines, -> { preload(pipelines: :user) } scope :last_of_each_version, -> { where(id: all.select('MAX(id) AS id').group(:version)) } scope :limit_recent, ->(limit) { order_created_desc.limit(limit) } diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index 34533a82ccd..072ff4a3a5f 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -3,6 +3,7 @@ class Packages::PackageFile < ApplicationRecord include UpdateProjectStatistics include FileStoreMounter include Packages::Installable + include Packages::Destructible INSTALLABLE_STATUSES = [:default].freeze @@ -11,7 +12,7 @@ class Packages::PackageFile < ApplicationRecord delegate :file_type, :dsc?, :component, :architecture, :fields, to: :debian_file_metadatum, prefix: :debian delegate :channel, :metadata, to: :helm_file_metadatum, prefix: :helm - enum status: { default: 0, pending_destruction: 1 } + enum status: { default: 0, pending_destruction: 1, processing: 2, error: 3 } belongs_to :package @@ -57,7 +58,7 @@ class Packages::PackageFile < ApplicationRecord .merge(project.packages.helm.installable) .joins(:helm_file_metadatum) .where(packages_helm_file_metadata: { channel: channel }) - result = result.installable if Feature.enabled?(:packages_installable_package_files) + result = result.installable if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) result end @@ -108,7 +109,7 @@ class Packages::PackageFile < ApplicationRecord cte_name = :packages_cte cte = Gitlab::SQL::CTE.new(cte_name, packages.select(:id)) - package_files = if Feature.enabled?(:packages_installable_package_files) + package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) ::Packages::PackageFile.installable.limit_recent(1) .where(arel_table[:package_id].eq(Arel.sql("#{cte_name}.id"))) else diff --git a/app/presenters/packages/conan/package_presenter.rb b/app/presenters/packages/conan/package_presenter.rb index 49f3c79a31a..57636922676 100644 --- a/app/presenters/packages/conan/package_presenter.rb +++ b/app/presenters/packages/conan/package_presenter.rb @@ -81,7 +81,7 @@ module Packages return unless @package strong_memoize(:package_files) do - if Feature.enabled?(:packages_installable_package_files) + if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) @package.installable_package_files.preload_conan_file_metadata else @package.package_files.preload_conan_file_metadata diff --git a/app/presenters/packages/detail/package_presenter.rb b/app/presenters/packages/detail/package_presenter.rb index 2a410002696..c257edcadfb 100644 --- a/app/presenters/packages/detail/package_presenter.rb +++ b/app/presenters/packages/detail/package_presenter.rb @@ -39,7 +39,7 @@ module Packages private def package_file_views - package_files = if Feature.enabled?(:packages_installable_package_files) + package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) @package.installable_package_files else @package.package_files diff --git a/app/presenters/packages/npm/package_presenter.rb b/app/presenters/packages/npm/package_presenter.rb index 6fb64970872..1f94187204f 100644 --- a/app/presenters/packages/npm/package_presenter.rb +++ b/app/presenters/packages/npm/package_presenter.rb @@ -26,7 +26,7 @@ module Packages .preload_npm_metadatum batched_packages.each do |package| - package_file = if Feature.enabled?(:packages_installable_package_files) + package_file = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) package.installable_package_files.last else package.package_files.last diff --git a/app/presenters/packages/nuget/presenter_helpers.rb b/app/presenters/packages/nuget/presenter_helpers.rb index 1f0a64e4e6e..cd3e123033c 100644 --- a/app/presenters/packages/nuget/presenter_helpers.rb +++ b/app/presenters/packages/nuget/presenter_helpers.rb @@ -27,7 +27,7 @@ module Packages end def archive_url_for(package) - package_files = if Feature.enabled?(:packages_installable_package_files) + package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) package.installable_package_files else package.package_files diff --git a/app/presenters/packages/pypi/package_presenter.rb b/app/presenters/packages/pypi/package_presenter.rb index a14e0f99cd9..33854e4d2fc 100644 --- a/app/presenters/packages/pypi/package_presenter.rb +++ b/app/presenters/packages/pypi/package_presenter.rb @@ -36,7 +36,7 @@ module Packages refs = [] @packages.map do |package| - package_files = if Feature.enabled?(:packages_installable_package_files) + package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) package.installable_package_files else package.package_files diff --git a/app/services/google_cloud/create_service_accounts_service.rb b/app/services/google_cloud/create_service_accounts_service.rb new file mode 100644 index 00000000000..fa025e8f672 --- /dev/null +++ b/app/services/google_cloud/create_service_accounts_service.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module GoogleCloud + class CreateServiceAccountsService < :: BaseService + def execute + service_account = google_api_client.create_service_account(gcp_project_id, service_account_name, service_account_desc) + service_account_key = google_api_client.create_service_account_key(gcp_project_id, service_account.unique_id) + + service_accounts_service.add_for_project( + environment_name, + service_account.project_id, + service_account.to_json, + service_account_key.to_json, + environment_protected? + ) + + ServiceResponse.success(message: _('Service account generated successfully'), payload: { + service_account: service_account, + service_account_key: service_account_key + }) + end + + private + + def google_oauth2_token + @params[:google_oauth2_token] + end + + def gcp_project_id + @params[:gcp_project_id] + end + + def environment_name + @params[:environment_name] + end + + def google_api_client + GoogleApi::CloudPlatform::Client.new(google_oauth2_token, nil) + end + + def service_accounts_service + GoogleCloud::ServiceAccountsService.new(project) + end + + def service_account_name + "GitLab :: #{project.name} :: #{environment_name}" + end + + def service_account_desc + "GitLab generated service account for project '#{project.name}' and environment '#{environment_name}'" + end + + # Overriden in EE + def environment_protected? + false + end + end +end + +GoogleCloud::CreateServiceAccountsService.prepend_mod diff --git a/app/services/google_cloud/service_accounts_service.rb b/app/services/google_cloud/service_accounts_service.rb index a512b27493d..3014daf08e2 100644 --- a/app/services/google_cloud/service_accounts_service.rb +++ b/app/services/google_cloud/service_accounts_service.rb @@ -27,39 +27,42 @@ module GoogleCloud end end - def add_for_project(environment, gcp_project_id, service_account, service_account_key) + def add_for_project(environment, gcp_project_id, service_account, service_account_key, is_protected) project_var_create_or_replace( environment, 'GCP_PROJECT_ID', - gcp_project_id + gcp_project_id, + is_protected ) project_var_create_or_replace( environment, 'GCP_SERVICE_ACCOUNT', - service_account + service_account, + is_protected ) project_var_create_or_replace( environment, 'GCP_SERVICE_ACCOUNT_KEY', - service_account_key + service_account_key, + is_protected ) end private def group_vars_by_environment - filtered_vars = @project.variables.filter { |variable| GCP_KEYS.include? variable.key } + filtered_vars = project.variables.filter { |variable| GCP_KEYS.include? variable.key } filtered_vars.each_with_object({}) do |variable, grouped| grouped[variable.environment_scope] ||= {} grouped[variable.environment_scope][variable.key] = variable.value end end - def project_var_create_or_replace(environment_scope, key, value) + def project_var_create_or_replace(environment_scope, key, value, is_protected) params = { key: key, filter: { environment_scope: environment_scope } } - existing_variable = ::Ci::VariablesFinder.new(@project, params).execute.first + existing_variable = ::Ci::VariablesFinder.new(project, params).execute.first existing_variable.destroy if existing_variable - @project.variables.create!(key: key, value: value, environment_scope: environment_scope, protected: true) + project.variables.create!(key: key, value: value, environment_scope: environment_scope, protected: is_protected) end end end diff --git a/app/services/packages/maven/metadata/sync_service.rb b/app/services/packages/maven/metadata/sync_service.rb index e60784a6489..4f35db36fb0 100644 --- a/app/services/packages/maven/metadata/sync_service.rb +++ b/app/services/packages/maven/metadata/sync_service.rb @@ -93,7 +93,7 @@ module Packages def metadata_package_file_for(package) return unless package - package_files = if Feature.enabled?(:packages_installable_package_files) + package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) package.installable_package_files else package.package_files diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 8ae06030d88..14fa6599073 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -444,6 +444,15 @@ :weight: 1 :idempotent: :tags: [] +- :name: cronjob:packages_cleanup_package_registry + :worker_name: Packages::CleanupPackageRegistryWorker + :feature_category: :package_registry + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: cronjob:packages_composer_cache_cleanup :worker_name: Packages::Composer::CacheCleanupWorker :feature_category: :package_registry @@ -1366,6 +1375,15 @@ :weight: 1 :idempotent: :tags: [] +- :name: package_cleanup:packages_cleanup_package_file + :worker_name: Packages::CleanupPackageFileWorker + :feature_category: :package_registry + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: package_repositories:packages_debian_generate_distribution :worker_name: Packages::Debian::GenerateDistributionWorker :feature_category: :package_registry diff --git a/app/workers/concerns/dependency_proxy/expireable.rb b/app/workers/concerns/dependency_proxy/expireable.rb index 9650ac85c6c..7e37db36bef 100644 --- a/app/workers/concerns/dependency_proxy/expireable.rb +++ b/app/workers/concerns/dependency_proxy/expireable.rb @@ -10,7 +10,7 @@ module DependencyProxy def expire_artifacts(collection) collection.each_batch(of: UPDATE_BATCH_SIZE) do |batch| - batch.update_all(status: :expired) + batch.update_all(status: :pending_destruction) end end end diff --git a/app/workers/concerns/dependency_proxy/cleanup_worker.rb b/app/workers/concerns/packages/cleanup_artifact_worker.rb index b668634f233..db6c7330ea3 100644 --- a/app/workers/concerns/dependency_proxy/cleanup_worker.rb +++ b/app/workers/concerns/packages/cleanup_artifact_worker.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true -module DependencyProxy - module CleanupWorker +module Packages + module CleanupArtifactWorker extend ActiveSupport::Concern + include LimitedCapacity::Worker include Gitlab::Utils::StrongMemoize def perform_work @@ -15,12 +16,8 @@ module DependencyProxy artifact&.error! end - def max_running_jobs - ::Gitlab::CurrentSettings.dependency_proxy_ttl_group_policy_worker_capacity - end - def remaining_work_count - expired_artifacts.limit(max_running_jobs + 1).count + artifacts_pending_destruction.limit(max_running_jobs + 1).count end private @@ -52,12 +49,12 @@ module DependencyProxy end end - def expired_artifacts - model.expired + def artifacts_pending_destruction + model.pending_destruction end def next_item - expired_artifacts.lock_next_by(:updated_at).first + model.next_pending_destruction(order_by: :updated_at) end end end diff --git a/app/workers/dependency_proxy/cleanup_blob_worker.rb b/app/workers/dependency_proxy/cleanup_blob_worker.rb index 054bc5854a3..eab536cd191 100644 --- a/app/workers/dependency_proxy/cleanup_blob_worker.rb +++ b/app/workers/dependency_proxy/cleanup_blob_worker.rb @@ -3,9 +3,8 @@ module DependencyProxy class CleanupBlobWorker include ApplicationWorker - include LimitedCapacity::Worker + include ::Packages::CleanupArtifactWorker include Gitlab::Utils::StrongMemoize - include DependencyProxy::CleanupWorker data_consistency :always @@ -17,6 +16,10 @@ module DependencyProxy worker_resource_boundary :unknown idempotent! + def max_running_jobs + ::Gitlab::CurrentSettings.dependency_proxy_ttl_group_policy_worker_capacity + end + private def model diff --git a/app/workers/dependency_proxy/cleanup_dependency_proxy_worker.rb b/app/workers/dependency_proxy/cleanup_dependency_proxy_worker.rb index d77c782267a..6a958d6e9d7 100644 --- a/app/workers/dependency_proxy/cleanup_dependency_proxy_worker.rb +++ b/app/workers/dependency_proxy/cleanup_dependency_proxy_worker.rb @@ -11,8 +11,8 @@ module DependencyProxy feature_category :dependency_proxy def perform - enqueue_blob_cleanup_job if DependencyProxy::Blob.expired.any? - enqueue_manifest_cleanup_job if DependencyProxy::Manifest.expired.any? + enqueue_blob_cleanup_job if DependencyProxy::Blob.pending_destruction.any? + enqueue_manifest_cleanup_job if DependencyProxy::Manifest.pending_destruction.any? end private diff --git a/app/workers/dependency_proxy/cleanup_manifest_worker.rb b/app/workers/dependency_proxy/cleanup_manifest_worker.rb index 1186efa2034..4445ee32e6d 100644 --- a/app/workers/dependency_proxy/cleanup_manifest_worker.rb +++ b/app/workers/dependency_proxy/cleanup_manifest_worker.rb @@ -3,9 +3,8 @@ module DependencyProxy class CleanupManifestWorker include ApplicationWorker - include LimitedCapacity::Worker + include ::Packages::CleanupArtifactWorker include Gitlab::Utils::StrongMemoize - include DependencyProxy::CleanupWorker data_consistency :always @@ -17,6 +16,10 @@ module DependencyProxy worker_resource_boundary :unknown idempotent! + def max_running_jobs + ::Gitlab::CurrentSettings.dependency_proxy_ttl_group_policy_worker_capacity + end + private def model diff --git a/app/workers/dependency_proxy/image_ttl_group_policy_worker.rb b/app/workers/dependency_proxy/image_ttl_group_policy_worker.rb index 3de2364fc71..89099ab6398 100644 --- a/app/workers/dependency_proxy/image_ttl_group_policy_worker.rb +++ b/app/workers/dependency_proxy/image_ttl_group_policy_worker.rb @@ -26,8 +26,8 @@ module DependencyProxy def log_counts use_replica_if_available do - expired_blob_count = DependencyProxy::Blob.expired.count - expired_manifest_count = DependencyProxy::Manifest.expired.count + expired_blob_count = DependencyProxy::Blob.pending_destruction.count + expired_manifest_count = DependencyProxy::Manifest.pending_destruction.count processing_blob_count = DependencyProxy::Blob.processing.count processing_manifest_count = DependencyProxy::Manifest.processing.count error_blob_count = DependencyProxy::Blob.error.count diff --git a/app/workers/packages/cleanup_package_file_worker.rb b/app/workers/packages/cleanup_package_file_worker.rb new file mode 100644 index 00000000000..cb2b2a12c5e --- /dev/null +++ b/app/workers/packages/cleanup_package_file_worker.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module Packages + class CleanupPackageFileWorker + include ApplicationWorker + include ::Packages::CleanupArtifactWorker + include Gitlab::Utils::StrongMemoize + + data_consistency :always + queue_namespace :package_cleanup + feature_category :package_registry + urgency :low + worker_resource_boundary :unknown + idempotent! + + def max_running_jobs + ::Gitlab::CurrentSettings.packages_cleanup_package_file_worker_capacity + end + + private + + def model + Packages::PackageFile + end + + def next_item + model.next_pending_destruction + end + + def log_metadata(package_file) + log_extra_metadata_on_done(:package_file_id, package_file.id) + log_extra_metadata_on_done(:package_id, package_file.package_id) + end + + def log_cleanup_item(package_file) + logger.info( + structured_payload( + package_id: package_file.package_id, + package_file_id: package_file.id + ) + ) + end + end +end diff --git a/app/workers/packages/cleanup_package_registry_worker.rb b/app/workers/packages/cleanup_package_registry_worker.rb new file mode 100644 index 00000000000..a849e055b64 --- /dev/null +++ b/app/workers/packages/cleanup_package_registry_worker.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Packages + class CleanupPackageRegistryWorker + include ApplicationWorker + include CronjobQueue # rubocop:disable Scalability/CronWorkerContext + + data_consistency :always + idempotent! + + feature_category :package_registry + + def perform + enqueue_package_file_cleanup_job if Packages::PackageFile.pending_destruction.exists? + + log_counts + end + + private + + def enqueue_package_file_cleanup_job + Packages::CleanupPackageFileWorker.perform_with_capacity + end + + def log_counts + use_replica_if_available do + pending_destruction_package_files_count = Packages::PackageFile.pending_destruction.count + processing_package_files_count = Packages::PackageFile.processing.count + error_package_files_count = Packages::PackageFile.error.count + + log_extra_metadata_on_done(:pending_destruction_package_files_count, pending_destruction_package_files_count) + log_extra_metadata_on_done(:processing_package_files_count, processing_package_files_count) + log_extra_metadata_on_done(:error_package_files_count, error_package_files_count) + end + end + + def use_replica_if_available(&block) + ::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&block) + end + end +end diff --git a/config/feature_flags/development/packages_installable_package_files.yml b/config/feature_flags/development/packages_installable_package_files.yml index a076f85f7cd..ed0091f41e2 100644 --- a/config/feature_flags/development/packages_installable_package_files.yml +++ b/config/feature_flags/development/packages_installable_package_files.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348677 milestone: '14.6' type: development group: group::package -default_enabled: false +default_enabled: true diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index f65c76d8b6b..ca5d133381c 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -545,6 +545,10 @@ Settings.cron_jobs['image_ttl_group_policy_worker']['job_class'] = 'DependencyPr Settings.cron_jobs['cleanup_dependency_proxy_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['cleanup_dependency_proxy_worker']['cron'] ||= '20 3,15 * * *' Settings.cron_jobs['cleanup_dependency_proxy_worker']['job_class'] = 'DependencyProxy::CleanupDependencyProxyWorker' +Settings.cron_jobs['cleanup_package_registry_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['cleanup_package_registry_worker']['cron'] ||= '20 0,12 * * *' +Settings.cron_jobs['cleanup_package_registry_worker']['job_class'] = 'Packages::CleanupPackageRegistryWorker' + Settings.cron_jobs['x509_issuer_crl_check_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['x509_issuer_crl_check_worker']['cron'] ||= '30 1 * * *' Settings.cron_jobs['x509_issuer_crl_check_worker']['job_class'] = 'X509IssuerCrlCheckWorker' diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 80c49660bdb..bf84527bc5f 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -297,6 +297,8 @@ - 1 - - object_storage - 1 +- - package_cleanup + - 1 - - package_repositories - 1 - - packages_composer_cache_update diff --git a/db/migrate/20211224112937_add_packages_cleanup_package_file_worker_capacity_to_application_settings.rb b/db/migrate/20211224112937_add_packages_cleanup_package_file_worker_capacity_to_application_settings.rb new file mode 100644 index 00000000000..cb58033361f --- /dev/null +++ b/db/migrate/20211224112937_add_packages_cleanup_package_file_worker_capacity_to_application_settings.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddPackagesCleanupPackageFileWorkerCapacityToApplicationSettings < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def change + add_column :application_settings, + :packages_cleanup_package_file_worker_capacity, + :smallint, + default: 2, + null: false + end +end diff --git a/db/migrate/20211224114539_add_packages_cleanup_package_file_worker_capacity_check_constraint_to_app_settings.rb b/db/migrate/20211224114539_add_packages_cleanup_package_file_worker_capacity_check_constraint_to_app_settings.rb new file mode 100644 index 00000000000..3a40540a1e1 --- /dev/null +++ b/db/migrate/20211224114539_add_packages_cleanup_package_file_worker_capacity_check_constraint_to_app_settings.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddPackagesCleanupPackageFileWorkerCapacityCheckConstraintToAppSettings < Gitlab::Database::Migration[1.0] + CONSTRAINT_NAME = 'app_settings_p_cleanup_package_file_worker_capacity_positive' + + disable_ddl_transaction! + + def up + add_check_constraint :application_settings, 'packages_cleanup_package_file_worker_capacity >= 0', CONSTRAINT_NAME + end + + def down + remove_check_constraint :application_settings, CONSTRAINT_NAME + end +end diff --git a/db/migrate/20220114131950_add_status_only_index_to_packages_package_files.rb b/db/migrate/20220114131950_add_status_only_index_to_packages_package_files.rb new file mode 100644 index 00000000000..948edea1138 --- /dev/null +++ b/db/migrate/20220114131950_add_status_only_index_to_packages_package_files.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddStatusOnlyIndexToPackagesPackageFiles < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + INDEX_NAME = 'index_packages_package_files_on_status' + + def up + add_concurrent_index :packages_package_files, :status, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :packages_package_files, name: INDEX_NAME + end +end diff --git a/db/post_migrate/20211208122200_schedule_backfill_ci_namespace_mirrors.rb b/db/post_migrate/20211208122200_schedule_backfill_ci_namespace_mirrors.rb new file mode 100644 index 00000000000..3d39148f402 --- /dev/null +++ b/db/post_migrate/20211208122200_schedule_backfill_ci_namespace_mirrors.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class ScheduleBackfillCiNamespaceMirrors < Gitlab::Database::Migration[1.0] + MIGRATION = 'BackfillCiNamespaceMirrors' + BATCH_SIZE = 10_000 + DELAY_INTERVAL = 2.minutes + + disable_ddl_transaction! + + def up + queue_background_migration_jobs_by_range_at_intervals( + Gitlab::BackgroundMigration::BackfillCiNamespaceMirrors::Namespace.base_query, + MIGRATION, + DELAY_INTERVAL, + batch_size: BATCH_SIZE, + track_jobs: true + ) + end + + def down + # no-op + end +end diff --git a/db/post_migrate/20211208122201_schedule_backfill_ci_project_mirrors.rb b/db/post_migrate/20211208122201_schedule_backfill_ci_project_mirrors.rb new file mode 100644 index 00000000000..5678ee9f292 --- /dev/null +++ b/db/post_migrate/20211208122201_schedule_backfill_ci_project_mirrors.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class ScheduleBackfillCiProjectMirrors < Gitlab::Database::Migration[1.0] + MIGRATION = 'BackfillCiProjectMirrors' + BATCH_SIZE = 10_000 + DELAY_INTERVAL = 2.minutes + + disable_ddl_transaction! + + def up + queue_background_migration_jobs_by_range_at_intervals( + Gitlab::BackgroundMigration::BackfillCiProjectMirrors::Project.base_query, + MIGRATION, + DELAY_INTERVAL, + batch_size: BATCH_SIZE, + track_jobs: true + ) + end + + def down + # no-op + end +end diff --git a/db/schema_migrations/20211208122200 b/db/schema_migrations/20211208122200 new file mode 100644 index 00000000000..1e4910d3a3c --- /dev/null +++ b/db/schema_migrations/20211208122200 @@ -0,0 +1 @@ +8dec4379f34773e979cf6b33ba608185827c37b1a95c74d6472b7562c16d6110
\ No newline at end of file diff --git a/db/schema_migrations/20211208122201 b/db/schema_migrations/20211208122201 new file mode 100644 index 00000000000..fb770a6d5fd --- /dev/null +++ b/db/schema_migrations/20211208122201 @@ -0,0 +1 @@ +f58ccb07fa67ede2a50fa5ff6bddbe4bb89a622e84786fc4bfb404c11b9dbab4
\ No newline at end of file diff --git a/db/schema_migrations/20211224112937 b/db/schema_migrations/20211224112937 new file mode 100644 index 00000000000..98296f9a270 --- /dev/null +++ b/db/schema_migrations/20211224112937 @@ -0,0 +1 @@ +3709c5c229e45ff0f594d6291d0b2b9a167b3bf4f5b29158b9abdac333a638b8
\ No newline at end of file diff --git a/db/schema_migrations/20211224114539 b/db/schema_migrations/20211224114539 new file mode 100644 index 00000000000..2d7494e4291 --- /dev/null +++ b/db/schema_migrations/20211224114539 @@ -0,0 +1 @@ +f4ac776ec4213d6fcd07ccfa913f51e1386ff212bf47d27817b24b501a78443b
\ No newline at end of file diff --git a/db/schema_migrations/20220114131950 b/db/schema_migrations/20220114131950 new file mode 100644 index 00000000000..ca1e1033d9b --- /dev/null +++ b/db/schema_migrations/20220114131950 @@ -0,0 +1 @@ +f427f4c0d75308f7c97685e10e27a735dcf284714acd49659f62a6f7752234ef
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 69910954a87..258597a21a4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10482,9 +10482,11 @@ CREATE TABLE application_settings ( static_objects_external_storage_auth_token_encrypted text, future_subscriptions jsonb DEFAULT '[]'::jsonb NOT NULL, user_email_lookup_limit integer DEFAULT 60 NOT NULL, + packages_cleanup_package_file_worker_capacity smallint DEFAULT 2 NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), + CONSTRAINT app_settings_p_cleanup_package_file_worker_capacity_positive CHECK ((packages_cleanup_package_file_worker_capacity >= 0)), CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)), CONSTRAINT app_settings_yaml_max_depth_positive CHECK ((max_yaml_depth > 0)), CONSTRAINT app_settings_yaml_max_size_positive CHECK ((max_yaml_size_bytes > 0)), @@ -27017,6 +27019,8 @@ CREATE INDEX index_packages_package_files_on_package_id_id ON packages_package_f CREATE INDEX index_packages_package_files_on_package_id_status_and_id ON packages_package_files USING btree (package_id, status, id); +CREATE INDEX index_packages_package_files_on_status ON packages_package_files USING btree (status); + CREATE INDEX index_packages_package_files_on_verification_state ON packages_package_files USING btree (verification_state); CREATE INDEX index_packages_packages_on_creator_id ON packages_packages USING btree (creator_id); diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index aacf117b3b7..905ecd05d52 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -9,6 +9,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w > - `reference` was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/20354) in GitLab 12.10 in favour of `references`. > - `reviewer_username` and `reviewer_id` were [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49341) in GitLab 13.8. > - `draft` was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63473) as a replacement for `work_in_progress` in GitLab 14.0. +> - `merge_user` was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/349031) as an eventual replacement for `merged_by` in GitLab 14.7. Every API call to merge requests must be authenticated. @@ -42,6 +43,11 @@ This approach is generally slower and more resource-intensive, but isn't subject placed on database-backed diffs. [Limits inherent to Gitaly](../development/diffs.md#diff-limits) still apply. +- [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/349031) in GitLab 14.7, +field `merge_user` can be either user who merged this merge request, +user who set it to merge when pipeline succeeds or `null`. +Field `merged_by` (user who merged this merge request or `null`) has been deprecated. + ## List merge requests Get all merge requests the authenticated user has access to. By @@ -111,7 +117,15 @@ Parameters: "title": "test1", "description": "fixed login page css paddings", "state": "merged", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", @@ -295,7 +309,15 @@ Parameters: "title": "test1", "description": "fixed login page css paddings", "state": "merged", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", @@ -481,7 +503,15 @@ Parameters: "title": "test1", "description": "fixed login page css paddings", "state": "merged", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", @@ -717,7 +747,15 @@ Parameters: "squash": false, "subscribed": false, "changes_count": "1", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", @@ -1159,7 +1197,15 @@ POST /projects/:id/merge_requests "squash": false, "subscribed": false, "changes_count": "1", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", @@ -1330,7 +1376,15 @@ Must include at least one non-required attribute from above. "squash": false, "subscribed": false, "changes_count": "1", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", @@ -1516,7 +1570,15 @@ Parameters: "squash": false, "subscribed": false, "changes_count": "1", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", @@ -1705,7 +1767,15 @@ Parameters: "squash": false, "subscribed": false, "changes_count": "1", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", @@ -2006,7 +2076,15 @@ Example response: "squash": false, "subscribed": false, "changes_count": "1", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", @@ -2166,7 +2244,15 @@ Example response: "squash": false, "subscribed": false, "changes_count": "1", - "merged_by": { + "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead + "id": 87854, + "name": "Douwe Maan", + "username": "DouweM", + "state": "active", + "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png", + "web_url": "https://gitlab.com/DouweM" + }, + "merge_user": { "id": 87854, "name": "Douwe Maan", "username": "DouweM", diff --git a/doc/development/testing_guide/ci.md b/doc/development/testing_guide/ci.md deleted file mode 100644 index de024084c9c..00000000000 --- a/doc/development/testing_guide/ci.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -redirect_to: '../pipelines.md' -remove_date: '2022-01-12' ---- - -This file was moved to [another location](../pipelines.md). - -<!-- This redirect file can be deleted after <2022-01-12>. --> -<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/security/rack_attack.md b/doc/security/rack_attack.md deleted file mode 100644 index a8b55007d2e..00000000000 --- a/doc/security/rack_attack.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -redirect_to: '../user/admin_area/settings/protected_paths.md' -remove_date: '2022-01-14' ---- - -This document was moved to [another location](../user/admin_area/settings/protected_paths.md). - -<!-- This redirect file can be deleted after <2022-01-14>. --> -<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/lib/api/entities/merge_request_basic.rb b/lib/api/entities/merge_request_basic.rb index d5cf2f653db..55d58166590 100644 --- a/lib/api/entities/merge_request_basic.rb +++ b/lib/api/entities/merge_request_basic.rb @@ -3,9 +3,13 @@ module API module Entities class MergeRequestBasic < IssuableEntity + # Deprecated in favour of merge_user expose :merged_by, using: Entities::UserBasic do |merge_request, _options| merge_request.metrics&.merged_by end + expose :merge_user, using: Entities::UserBasic do |merge_request| + merge_request.metrics&.merged_by || merge_request.merge_user + end expose :merged_at do |merge_request, _options| merge_request.metrics&.merged_at end diff --git a/lib/api/package_files.rb b/lib/api/package_files.rb index 40f43b4adf7..5e421da2c55 100644 --- a/lib/api/package_files.rb +++ b/lib/api/package_files.rb @@ -28,7 +28,7 @@ module API package = ::Packages::PackageFinder .new(user_project, params[:package_id]).execute - package_files = if Feature.enabled?(:packages_installable_package_files) + package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) package.installable_package_files else package.package_files @@ -55,7 +55,7 @@ module API not_found! unless package - package_files = if Feature.enabled?(:packages_installable_package_files) + package_files = if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) package.installable_package_files else package.package_files diff --git a/lib/api/rubygem_packages.rb b/lib/api/rubygem_packages.rb index c7f380c9a69..3effa370e84 100644 --- a/lib/api/rubygem_packages.rb +++ b/lib/api/rubygem_packages.rb @@ -69,7 +69,7 @@ module API package_files = ::Packages::PackageFile .for_rubygem_with_file_name(user_project, params[:file_name]) - package_files = package_files.installable if Feature.enabled?(:packages_installable_package_files) + package_files = package_files.installable if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) package_file = package_files.last! diff --git a/lib/api/terraform/modules/v1/packages.rb b/lib/api/terraform/modules/v1/packages.rb index e27c5c22d1a..970fdeba734 100644 --- a/lib/api/terraform/modules/v1/packages.rb +++ b/lib/api/terraform/modules/v1/packages.rb @@ -71,7 +71,7 @@ module API def package_file strong_memoize(:package_file) do - if Feature.enabled?(:packages_installable_package_files) + if Feature.enabled?(:packages_installable_package_files, default_enabled: :yaml) package.installable_package_files.first else package.package_files.first diff --git a/lib/gitlab/background_migration/backfill_ci_namespace_mirrors.rb b/lib/gitlab/background_migration/backfill_ci_namespace_mirrors.rb new file mode 100644 index 00000000000..2247747ba08 --- /dev/null +++ b/lib/gitlab/background_migration/backfill_ci_namespace_mirrors.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # A job to create ci_namespace_mirrors entries in batches + class BackfillCiNamespaceMirrors + class Namespace < ActiveRecord::Base # rubocop:disable Style/Documentation + include ::EachBatch + + self.table_name = 'namespaces' + self.inheritance_column = nil + + scope :base_query, -> do + select(:id, :parent_id) + end + end + + PAUSE_SECONDS = 0.1 + SUB_BATCH_SIZE = 500 + + def perform(start_id, end_id) + batch_query = Namespace.base_query.where(id: start_id..end_id) + batch_query.each_batch(of: SUB_BATCH_SIZE) do |sub_batch| + first, last = sub_batch.pluck(Arel.sql('MIN(id), MAX(id)')).first + ranged_query = Namespace.unscoped.base_query.where(id: first..last) + + update_sql = <<~SQL + INSERT INTO ci_namespace_mirrors (namespace_id, traversal_ids) + #{insert_values(ranged_query)} + ON CONFLICT (namespace_id) DO NOTHING + SQL + # We do nothing on conflict because we consider they were already filled. + + Namespace.connection.execute(update_sql) + + sleep PAUSE_SECONDS + end + + mark_job_as_succeeded(start_id, end_id) + end + + private + + def insert_values(batch) + calculated_traversal_ids( + batch.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') + ) + end + + # Copied from lib/gitlab/background_migration/backfill_namespace_traversal_ids_children.rb + def calculated_traversal_ids(batch) + <<~SQL + WITH RECURSIVE cte(source_id, namespace_id, parent_id, height) AS ( + ( + SELECT batch.id, batch.id, batch.parent_id, 1 + FROM (#{batch.to_sql}) AS batch + ) + UNION ALL + ( + SELECT cte.source_id, n.id, n.parent_id, cte.height+1 + FROM namespaces n, cte + WHERE n.id = cte.parent_id + ) + ) + SELECT flat_hierarchy.source_id as namespace_id, + array_agg(flat_hierarchy.namespace_id ORDER BY flat_hierarchy.height DESC) as traversal_ids + FROM (SELECT * FROM cte FOR UPDATE) flat_hierarchy + GROUP BY flat_hierarchy.source_id + SQL + end + + def mark_job_as_succeeded(*arguments) + Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded('BackfillCiNamespaceMirrors', arguments) + end + end + end +end diff --git a/lib/gitlab/background_migration/backfill_ci_project_mirrors.rb b/lib/gitlab/background_migration/backfill_ci_project_mirrors.rb new file mode 100644 index 00000000000..ff6ab9928b0 --- /dev/null +++ b/lib/gitlab/background_migration/backfill_ci_project_mirrors.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # A job to create ci_project_mirrors entries in batches + class BackfillCiProjectMirrors + class Project < ActiveRecord::Base # rubocop:disable Style/Documentation + include ::EachBatch + + self.table_name = 'projects' + + scope :base_query, -> do + select(:id, :namespace_id) + end + end + + PAUSE_SECONDS = 0.1 + SUB_BATCH_SIZE = 500 + + def perform(start_id, end_id) + batch_query = Project.base_query.where(id: start_id..end_id) + batch_query.each_batch(of: SUB_BATCH_SIZE) do |sub_batch| + first, last = sub_batch.pluck(Arel.sql('MIN(id), MAX(id)')).first + ranged_query = Project.unscoped.base_query.where(id: first..last) + + update_sql = <<~SQL + INSERT INTO ci_project_mirrors (project_id, namespace_id) + #{insert_values(ranged_query)} + ON CONFLICT (project_id) DO NOTHING + SQL + # We do nothing on conflict because we consider they were already filled. + + Project.connection.execute(update_sql) + + sleep PAUSE_SECONDS + end + + mark_job_as_succeeded(start_id, end_id) + end + + private + + def insert_values(batch) + batch.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433').to_sql + end + + def mark_job_as_succeeded(*arguments) + Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded('BackfillCiProjectMirrors', arguments) + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 63cfffb0f2d..4b28f3b6b2f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -33187,12 +33187,21 @@ msgstr "" msgid "SlackIntegration|GitLab for Slack" msgstr "" +msgid "SlackIntegration|Project alias" +msgstr "" + msgid "SlackIntegration|Select a GitLab project to link with your Slack workspace." msgstr "" msgid "SlackIntegration|Sends notifications about project events to Slack channels." msgstr "" +msgid "SlackIntegration|Team name" +msgstr "" + +msgid "SlackIntegration|To set up this integration press \"Add to Slack\"" +msgstr "" + msgid "SlackService|1. %{slash_command_link_start}Add a slash command%{slash_command_link_end} in your Slack team using this information:" msgstr "" diff --git a/spec/factories/dependency_proxy.rb b/spec/factories/dependency_proxy.rb index 836ee87e4d7..afa6c61116a 100644 --- a/spec/factories/dependency_proxy.rb +++ b/spec/factories/dependency_proxy.rb @@ -8,8 +8,8 @@ FactoryBot.define do file_name { 'a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4.gz' } status { :default } - trait :expired do - status { :expired } + trait :pending_destruction do + status { :pending_destruction } end end @@ -22,8 +22,8 @@ FactoryBot.define do content_type { 'application/vnd.docker.distribution.manifest.v2+json' } status { :default } - trait :expired do - status { :expired } + trait :pending_destruction do + status { :pending_destruction } end end end diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_request.json b/spec/fixtures/api/schemas/public_api/v4/merge_request.json index c31e91cfef8..a55c4b8974b 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_request.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_request.json @@ -20,6 +20,18 @@ }, "additionalProperties": false }, + "merge_user": { + "type": ["object", "null"], + "properties": { + "name": { "type": "string" }, + "username": { "type": "string" }, + "id": { "type": "integer" }, + "state": { "type": "string" }, + "avatar_url": { "type": "uri" }, + "web_url": { "type": "uri" } + }, + "additionalProperties": false + }, "merged_at": { "type": ["string", "null"] }, "closed_by": { "type": ["object", "null"], diff --git a/spec/lib/api/entities/merge_request_basic_spec.rb b/spec/lib/api/entities/merge_request_basic_spec.rb index b9d6ab7a652..40f259b86e2 100644 --- a/spec/lib/api/entities/merge_request_basic_spec.rb +++ b/spec/lib/api/entities/merge_request_basic_spec.rb @@ -21,7 +21,8 @@ RSpec.describe ::API::Entities::MergeRequestBasic do it 'includes basic fields' do is_expected.to include( draft: merge_request.draft?, - work_in_progress: merge_request.draft? + work_in_progress: merge_request.draft?, + merge_user: nil ) end diff --git a/spec/lib/gitlab/background_migration/backfill_ci_namespace_mirrors_spec.rb b/spec/lib/gitlab/background_migration/backfill_ci_namespace_mirrors_spec.rb new file mode 100644 index 00000000000..8980a26932b --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_ci_namespace_mirrors_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillCiNamespaceMirrors, :migration, schema: 20211208122200 do + let(:namespaces) { table(:namespaces) } + let(:ci_namespace_mirrors) { table(:ci_namespace_mirrors) } + + subject { described_class.new } + + describe '#perform' do + it 'creates hierarchies for all namespaces in range' do + namespaces.create!(id: 5, name: 'test1', path: 'test1') + namespaces.create!(id: 7, name: 'test2', path: 'test2') + namespaces.create!(id: 8, name: 'test3', path: 'test3') + + subject.perform(5, 7) + + expect(ci_namespace_mirrors.all).to contain_exactly( + an_object_having_attributes(namespace_id: 5, traversal_ids: [5]), + an_object_having_attributes(namespace_id: 7, traversal_ids: [7]) + ) + end + + it 'handles existing hierarchies gracefully' do + namespaces.create!(id: 5, name: 'test1', path: 'test1') + test2 = namespaces.create!(id: 7, name: 'test2', path: 'test2') + namespaces.create!(id: 8, name: 'test3', path: 'test3', parent_id: 7) + namespaces.create!(id: 9, name: 'test4', path: 'test4') + + # Simulate a situation where a user has had a chance to move a group to another parent + # before the background migration has had a chance to run + test2.update!(parent_id: 5) + ci_namespace_mirrors.create!(namespace_id: test2.id, traversal_ids: [5, 7]) + + subject.perform(5, 8) + + expect(ci_namespace_mirrors.all).to contain_exactly( + an_object_having_attributes(namespace_id: 5, traversal_ids: [5]), + an_object_having_attributes(namespace_id: 7, traversal_ids: [5, 7]), + an_object_having_attributes(namespace_id: 8, traversal_ids: [5, 7, 8]) + ) + end + end +end diff --git a/spec/lib/gitlab/background_migration/backfill_ci_project_mirrors_spec.rb b/spec/lib/gitlab/background_migration/backfill_ci_project_mirrors_spec.rb new file mode 100644 index 00000000000..4eec83879e3 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_ci_project_mirrors_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillCiProjectMirrors, :migration, schema: 20211208122201 do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:ci_project_mirrors) { table(:ci_project_mirrors) } + + subject { described_class.new } + + describe '#perform' do + it 'creates ci_project_mirrors for all projects in range' do + namespaces.create!(id: 10, name: 'namespace1', path: 'namespace1') + projects.create!(id: 5, namespace_id: 10, name: 'test1', path: 'test1') + projects.create!(id: 7, namespace_id: 10, name: 'test2', path: 'test2') + projects.create!(id: 8, namespace_id: 10, name: 'test3', path: 'test3') + + subject.perform(5, 7) + + expect(ci_project_mirrors.all).to contain_exactly( + an_object_having_attributes(project_id: 5, namespace_id: 10), + an_object_having_attributes(project_id: 7, namespace_id: 10) + ) + end + + it 'handles existing ci_project_mirrors gracefully' do + namespaces.create!(id: 10, name: 'namespace1', path: 'namespace1') + namespaces.create!(id: 11, name: 'namespace2', path: 'namespace2', parent_id: 10) + projects.create!(id: 5, namespace_id: 10, name: 'test1', path: 'test1') + projects.create!(id: 7, namespace_id: 11, name: 'test2', path: 'test2') + projects.create!(id: 8, namespace_id: 11, name: 'test3', path: 'test3') + + # Simulate a situation where a user has had a chance to move a project to another namespace + # before the background migration has had a chance to run + ci_project_mirrors.create!(project_id: 7, namespace_id: 10) + + subject.perform(5, 7) + + expect(ci_project_mirrors.all).to contain_exactly( + an_object_having_attributes(project_id: 5, namespace_id: 10), + an_object_having_attributes(project_id: 7, namespace_id: 10) + ) + end + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 81728432a30..8cc763b0201 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -80,6 +80,9 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_numericality_of(:dependency_proxy_ttl_group_policy_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.not_to allow_value(nil).for(:dependency_proxy_ttl_group_policy_worker_capacity) } + it { is_expected.to validate_numericality_of(:packages_cleanup_package_file_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.not_to allow_value(nil).for(:packages_cleanup_package_file_worker_capacity) } + it { is_expected.to validate_numericality_of(:snippet_size_limit).only_integer.is_greater_than(0) } it { is_expected.to validate_numericality_of(:wiki_page_max_content_bytes).only_integer.is_greater_than_or_equal_to(1024) } it { is_expected.to validate_presence_of(:max_artifacts_size) } diff --git a/spec/models/dependency_proxy/blob_spec.rb b/spec/models/dependency_proxy/blob_spec.rb index 3c54d3126a8..10d06406ad7 100644 --- a/spec/models/dependency_proxy/blob_spec.rb +++ b/spec/models/dependency_proxy/blob_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe DependencyProxy::Blob, type: :model do it_behaves_like 'ttl_expirable' + it_behaves_like 'destructible', factory: :dependency_proxy_blob describe 'relationships' do it { is_expected.to belong_to(:group) } diff --git a/spec/models/dependency_proxy/manifest_spec.rb b/spec/models/dependency_proxy/manifest_spec.rb index 59415096989..ab7881b1d39 100644 --- a/spec/models/dependency_proxy/manifest_spec.rb +++ b/spec/models/dependency_proxy/manifest_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe DependencyProxy::Manifest, type: :model do it_behaves_like 'ttl_expirable' + it_behaves_like 'destructible', factory: :dependency_proxy_manifest describe 'relationships' do it { is_expected.to belong_to(:group) } diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index 5b5cb0827a8..a86caa074f1 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Packages::PackageFile, type: :model do let_it_be(:debian_package) { create(:debian_package, project: project) } it_behaves_like 'having unique enum values' + it_behaves_like 'destructible', factory: :package_file describe 'relationships' do it { is_expected.to belong_to(:package) } diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index d5f0fdeacd7..a751f785913 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1269,6 +1269,7 @@ RSpec.describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) expect(json_response).to include('merged_by', + 'merge_user', 'merged_at', 'closed_by', 'closed_at', @@ -1279,9 +1280,10 @@ RSpec.describe API::MergeRequests do end it 'returns correct values' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.reload.iid}", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) expect(json_response['merged_by']['id']).to eq(merge_request.metrics.merged_by_id) + expect(json_response['merge_user']['id']).to eq(merge_request.metrics.merged_by_id) expect(Time.parse(json_response['merged_at'])).to be_like_time(merge_request.metrics.merged_at) expect(json_response['closed_by']['id']).to eq(merge_request.metrics.latest_closed_by_id) expect(Time.parse(json_response['closed_at'])).to be_like_time(merge_request.metrics.latest_closed_at) @@ -1292,6 +1294,32 @@ RSpec.describe API::MergeRequests do end end + context 'merge_user' do + context 'when MR is set to MWPS' do + let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds, source_project: project, target_project: project) } + + it 'returns user who set MWPS' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['merge_user']['id']).to eq(user.id) + end + + context 'when MR is already merged' do + before do + merge_request.metrics.update!(merged_by: user2) + end + + it 'returns user who actually merged' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['merge_user']['id']).to eq(user2.id) + end + end + end + end + context 'head_pipeline' do let(:project) { create(:project, :repository) } let(:merge_request) { create(:merge_request, :simple, author: user, source_project: project, source_branch: 'markdown', title: "Test") } diff --git a/spec/services/dependency_proxy/find_cached_manifest_service_spec.rb b/spec/services/dependency_proxy/find_cached_manifest_service_spec.rb index 29bdf1f11c3..607d67d8efe 100644 --- a/spec/services/dependency_proxy/find_cached_manifest_service_spec.rb +++ b/spec/services/dependency_proxy/find_cached_manifest_service_spec.rb @@ -91,9 +91,9 @@ RSpec.describe DependencyProxy::FindCachedManifestService do it_behaves_like 'returning no manifest' end - context 'when the cached manifest is expired' do + context 'when the cached manifest is pending destruction' do before do - dependency_proxy_manifest.update_column(:status, DependencyProxy::Manifest.statuses[:expired]) + dependency_proxy_manifest.update_column(:status, DependencyProxy::Manifest.statuses[:pending_destruction]) stub_manifest_head(image, tag, headers: headers) stub_manifest_download(image, tag, headers: headers) end diff --git a/spec/services/google_cloud/create_service_accounts_service_spec.rb b/spec/services/google_cloud/create_service_accounts_service_spec.rb new file mode 100644 index 00000000000..190e1a8098c --- /dev/null +++ b/spec/services/google_cloud/create_service_accounts_service_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Mock Types +MockGoogleOAuth2Credentials = Struct.new(:app_id, :app_secret) +MockServiceAccount = Struct.new(:project_id, :unique_id) + +RSpec.describe GoogleCloud::CreateServiceAccountsService do + describe '#execute' do + before do + allow(Gitlab::Auth::OAuth::Provider).to receive(:config_for) + .with('google_oauth2') + .and_return(MockGoogleOAuth2Credentials.new('mock-app-id', 'mock-app-secret')) + + allow_next_instance_of(GoogleApi::CloudPlatform::Client) do |client| + allow(client).to receive(:create_service_account) + .and_return(MockServiceAccount.new('mock-project-id', 'mock-unique-id')) + allow(client).to receive(:create_service_account_key) + .and_return('mock-key') + end + end + + it 'creates unprotected vars', :aggregate_failures do + project = create(:project) + + service = described_class.new( + project, + nil, + google_oauth2_token: 'mock-token', + gcp_project_id: 'mock-gcp-project-id', + environment_name: '*' + ) + + response = service.execute + + expect(response.status).to eq(:success) + expect(response.message).to eq('Service account generated successfully') + expect(project.variables.count).to eq(3) + expect(project.variables.first.protected).to eq(false) + expect(project.variables.second.protected).to eq(false) + expect(project.variables.third.protected).to eq(false) + end + end +end diff --git a/spec/services/google_cloud/service_accounts_service_spec.rb b/spec/services/google_cloud/service_accounts_service_spec.rb index 505c623c02a..17c1f61a96e 100644 --- a/spec/services/google_cloud/service_accounts_service_spec.rb +++ b/spec/services/google_cloud/service_accounts_service_spec.rb @@ -60,8 +60,8 @@ RSpec.describe GoogleCloud::ServiceAccountsService do let_it_be(:project) { create(:project) } it 'saves GCP creds as project CI vars' do - service.add_for_project('env_1', 'gcp_prj_id_1', 'srv_acc_1', 'srv_acc_key_1') - service.add_for_project('env_2', 'gcp_prj_id_2', 'srv_acc_2', 'srv_acc_key_2') + service.add_for_project('env_1', 'gcp_prj_id_1', 'srv_acc_1', 'srv_acc_key_1', true) + service.add_for_project('env_2', 'gcp_prj_id_2', 'srv_acc_2', 'srv_acc_key_2', false) list = service.find_for_project @@ -81,7 +81,7 @@ RSpec.describe GoogleCloud::ServiceAccountsService do end it 'replaces previously stored CI vars with new CI vars' do - service.add_for_project('env_1', 'new_project', 'srv_acc_1', 'srv_acc_key_1') + service.add_for_project('env_1', 'new_project', 'srv_acc_1', 'srv_acc_key_1', false) list = service.find_for_project @@ -101,9 +101,16 @@ RSpec.describe GoogleCloud::ServiceAccountsService do end end - it 'underlying project CI vars must be protected' do - expect(project.variables.first.protected).to eq(true) - expect(project.variables.second.protected).to eq(true) + it 'underlying project CI vars must be protected as per value' do + service.add_for_project('env_1', 'gcp_prj_id_1', 'srv_acc_1', 'srv_acc_key_1', true) + service.add_for_project('env_2', 'gcp_prj_id_2', 'srv_acc_2', 'srv_acc_key_2', false) + + expect(project.variables[0].protected).to eq(true) + expect(project.variables[1].protected).to eq(true) + expect(project.variables[2].protected).to eq(true) + expect(project.variables[3].protected).to eq(false) + expect(project.variables[4].protected).to eq(false) + expect(project.variables[5].protected).to eq(false) end end end diff --git a/spec/support/shared_examples/models/concerns/packages/destructible_shared_examples.rb b/spec/support/shared_examples/models/concerns/packages/destructible_shared_examples.rb new file mode 100644 index 00000000000..f974b46f881 --- /dev/null +++ b/spec/support/shared_examples/models/concerns/packages/destructible_shared_examples.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'destructible' do |factory:| + let_it_be(:item1) { create(factory, created_at: 1.month.ago, updated_at: 1.day.ago) } + let_it_be(:item2) { create(factory, created_at: 1.year.ago, updated_at: 1.year.ago) } + let_it_be(:item3) { create(factory, :pending_destruction, created_at: 2.years.ago, updated_at: 1.month.ago) } + let_it_be(:item4) { create(factory, :pending_destruction, created_at: 3.years.ago, updated_at: 2.weeks.ago) } + + describe '.next_pending_destruction' do + it 'returns the oldest item pending destruction based on updated_at' do + expect(described_class.next_pending_destruction(order_by: :updated_at)).to eq(item3) + end + + it 'returns the oldest item pending destruction based on created_at' do + expect(described_class.next_pending_destruction(order_by: :created_at)).to eq(item4) + end + end +end diff --git a/spec/support/shared_examples/models/concerns/ttl_expirable_shared_examples.rb b/spec/support/shared_examples/models/concerns/ttl_expirable_shared_examples.rb index 2d08de297a3..174b8609337 100644 --- a/spec/support/shared_examples/models/concerns/ttl_expirable_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/ttl_expirable_shared_examples.rb @@ -29,7 +29,7 @@ RSpec.shared_examples 'ttl_expirable' do describe '.active' do # rubocop:disable Rails/SaveBang let_it_be(:item1) { create(class_symbol) } - let_it_be(:item2) { create(class_symbol, :expired) } + let_it_be(:item2) { create(class_symbol, :pending_destruction) } let_it_be(:item3) { create(class_symbol, status: :error) } # rubocop:enable Rails/SaveBang @@ -38,17 +38,6 @@ RSpec.shared_examples 'ttl_expirable' do end end - describe '.lock_next_by' do - let_it_be(:item1) { create(class_symbol, created_at: 1.month.ago, updated_at: 1.day.ago) } - let_it_be(:item2) { create(class_symbol, created_at: 1.year.ago, updated_at: 1.year.ago) } - let_it_be(:item3) { create(class_symbol, created_at: 2.years.ago, updated_at: 1.month.ago) } - - it 'returns the first item sorted by the argument' do - expect(described_class.lock_next_by(:updated_at)).to contain_exactly(item2) - expect(described_class.lock_next_by(:created_at)).to contain_exactly(item3) - end - end - describe '#read', :freeze_time do let_it_be(:old_read_at) { 1.day.ago } let_it_be(:item1) { create(class_symbol, read_at: old_read_at) } diff --git a/spec/support/shared_examples/workers/concerns/dependency_proxy/cleanup_worker_shared_examples.rb b/spec/support/shared_examples/workers/concerns/dependency_proxy/cleanup_worker_shared_examples.rb index c9014ad549c..26444437826 100644 --- a/spec/support/shared_examples/workers/concerns/dependency_proxy/cleanup_worker_shared_examples.rb +++ b/spec/support/shared_examples/workers/concerns/dependency_proxy/cleanup_worker_shared_examples.rb @@ -13,12 +13,12 @@ RSpec.shared_examples 'dependency_proxy_cleanup_worker' do end context 'with work to do' do - let_it_be(:artifact1) { create(factory_type, :expired, group: group) } - let_it_be(:artifact2) { create(factory_type, :expired, group: group, updated_at: 6.months.ago, created_at: 2.years.ago) } - let_it_be_with_reload(:artifact3) { create(factory_type, :expired, group: group, updated_at: 1.year.ago, created_at: 1.year.ago) } + let_it_be(:artifact1) { create(factory_type, :pending_destruction, group: group) } + let_it_be(:artifact2) { create(factory_type, :pending_destruction, group: group, updated_at: 6.months.ago, created_at: 2.years.ago) } + let_it_be_with_reload(:artifact3) { create(factory_type, :pending_destruction, group: group, updated_at: 1.year.ago, created_at: 1.year.ago) } let_it_be(:artifact4) { create(factory_type, group: group, updated_at: 2.years.ago, created_at: 2.years.ago) } - it 'deletes the oldest expired artifact based on updated_at', :aggregate_failures do + it 'deletes the oldest artifact pending destruction based on updated_at', :aggregate_failures do expect(worker).to receive(:log_extra_metadata_on_done).with("#{factory_type}_id".to_sym, artifact3.id) expect(worker).to receive(:log_extra_metadata_on_done).with(:group_id, group.id) @@ -40,10 +40,8 @@ RSpec.shared_examples 'dependency_proxy_cleanup_worker' do end describe '#remaining_work_count' do - let_it_be(:expired_artifacts) do - (1..3).map do |_| - create(factory_type, :expired, group: group) - end + before(:context) do + create_list(factory_type, 3, :pending_destruction, group: group) end subject { worker.remaining_work_count } diff --git a/spec/workers/concerns/packages/cleanup_artifact_worker_spec.rb b/spec/workers/concerns/packages/cleanup_artifact_worker_spec.rb new file mode 100644 index 00000000000..95962d4810e --- /dev/null +++ b/spec/workers/concerns/packages/cleanup_artifact_worker_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Packages::CleanupArtifactWorker do + let_it_be(:worker_class) do + Class.new do + def self.name + 'Gitlab::Foo::Bar::DummyWorker' + end + + include ApplicationWorker + include ::Packages::CleanupArtifactWorker + end + end + + let(:worker) { worker_class.new } + + describe '#model' do + subject { worker.send(:model) } + + it { expect { subject }.to raise_error(NotImplementedError) } + end + + describe '#log_metadata' do + subject { worker.send(:log_metadata) } + + it { expect { subject }.to raise_error(NotImplementedError) } + end + + describe '#log_cleanup_item' do + subject { worker.send(:log_cleanup_item) } + + it { expect { subject }.to raise_error(NotImplementedError) } + end +end diff --git a/spec/workers/dependency_proxy/cleanup_dependency_proxy_worker_spec.rb b/spec/workers/dependency_proxy/cleanup_dependency_proxy_worker_spec.rb index ed0bdefbdb8..1100f9a7fae 100644 --- a/spec/workers/dependency_proxy/cleanup_dependency_proxy_worker_spec.rb +++ b/spec/workers/dependency_proxy/cleanup_dependency_proxy_worker_spec.rb @@ -9,8 +9,8 @@ RSpec.describe DependencyProxy::CleanupDependencyProxyWorker do context 'when there are records to be deleted' do it_behaves_like 'an idempotent worker' do it 'queues the cleanup jobs', :aggregate_failures do - create(:dependency_proxy_blob, :expired) - create(:dependency_proxy_manifest, :expired) + create(:dependency_proxy_blob, :pending_destruction) + create(:dependency_proxy_manifest, :pending_destruction) expect(DependencyProxy::CleanupBlobWorker).to receive(:perform_with_capacity).twice expect(DependencyProxy::CleanupManifestWorker).to receive(:perform_with_capacity).twice diff --git a/spec/workers/dependency_proxy/image_ttl_group_policy_worker_spec.rb b/spec/workers/dependency_proxy/image_ttl_group_policy_worker_spec.rb index b035a2ec0b7..6a2fdfbe8f5 100644 --- a/spec/workers/dependency_proxy/image_ttl_group_policy_worker_spec.rb +++ b/spec/workers/dependency_proxy/image_ttl_group_policy_worker_spec.rb @@ -17,19 +17,19 @@ RSpec.describe DependencyProxy::ImageTtlGroupPolicyWorker do let_it_be_with_reload(:new_blob) { create(:dependency_proxy_blob, group: group) } let_it_be_with_reload(:new_manifest) { create(:dependency_proxy_manifest, group: group) } - it 'updates the old images to expired' do + it 'updates the old images to pending_destruction' do expect { subject } - .to change { old_blob.reload.status }.from('default').to('expired') - .and change { old_manifest.reload.status }.from('default').to('expired') + .to change { old_blob.reload.status }.from('default').to('pending_destruction') + .and change { old_manifest.reload.status }.from('default').to('pending_destruction') .and not_change { new_blob.reload.status } .and not_change { new_manifest.reload.status } end end context 'counts logging' do - let_it_be(:expired_blob) { create(:dependency_proxy_blob, :expired, group: group) } - let_it_be(:expired_blob2) { create(:dependency_proxy_blob, :expired, group: group) } - let_it_be(:expired_manifest) { create(:dependency_proxy_manifest, :expired, group: group) } + let_it_be(:expired_blob) { create(:dependency_proxy_blob, :pending_destruction, group: group) } + let_it_be(:expired_blob2) { create(:dependency_proxy_blob, :pending_destruction, group: group) } + let_it_be(:expired_manifest) { create(:dependency_proxy_manifest, :pending_destruction, group: group) } let_it_be(:processing_blob) { create(:dependency_proxy_blob, status: :processing, group: group) } let_it_be(:processing_manifest) { create(:dependency_proxy_manifest, status: :processing, group: group) } let_it_be(:error_blob) { create(:dependency_proxy_blob, status: :error, group: group) } diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 30bde2a2791..bb4e2981070 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -361,6 +361,7 @@ RSpec.describe 'Every Sidekiq worker' do 'ObjectPool::ScheduleJoinWorker' => 3, 'ObjectStorage::BackgroundMoveWorker' => 5, 'ObjectStorage::MigrateUploadsWorker' => 3, + 'Packages::CleanupPackageFileWorker' => 0, 'Packages::Composer::CacheUpdateWorker' => false, 'Packages::Go::SyncPackagesWorker' => 3, 'Packages::Maven::Metadata::SyncWorker' => 3, diff --git a/spec/workers/packages/cleanup_package_file_worker_spec.rb b/spec/workers/packages/cleanup_package_file_worker_spec.rb new file mode 100644 index 00000000000..4dd06f13aff --- /dev/null +++ b/spec/workers/packages/cleanup_package_file_worker_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::CleanupPackageFileWorker do + let_it_be(:package) { create(:package) } + + let(:worker) { described_class.new } + + describe '#perform_work' do + subject { worker.perform_work } + + context 'with no work to do' do + it { is_expected.to be_nil } + end + + context 'with work to do' do + let_it_be(:package_file1) { create(:package_file, package: package) } + let_it_be(:package_file2) { create(:package_file, :pending_destruction, package: package) } + let_it_be(:package_file3) { create(:package_file, :pending_destruction, package: package, updated_at: 1.year.ago, created_at: 1.year.ago) } + + it 'deletes the oldest package file pending destruction based on id', :aggregate_failures do + # NOTE: The worker doesn't explicitly look for the lower id value, but this is how PostgreSQL works when + # using LIMIT without ORDER BY. + expect(worker).to receive(:log_extra_metadata_on_done).with(:package_file_id, package_file2.id) + expect(worker).to receive(:log_extra_metadata_on_done).with(:package_id, package.id) + + expect { subject }.to change { Packages::PackageFile.count }.by(-1) + end + end + + context 'with an error during the destroy' do + let_it_be(:package_file) { create(:package_file, :pending_destruction) } + + before do + expect(worker).to receive(:log_metadata).and_raise('Error!') + end + + it 'handles the error' do + expect { subject }.to change { Packages::PackageFile.error.count }.from(0).to(1) + expect(package_file.reload).to be_error + end + end + end + + describe '#max_running_jobs' do + let(:capacity) { 5 } + + subject { worker.max_running_jobs } + + before do + stub_application_setting(packages_cleanup_package_file_worker_capacity: capacity) + end + + it { is_expected.to eq(capacity) } + end + + describe '#remaining_work_count' do + before(:context) do + create_list(:package_file, 3, :pending_destruction, package: package) + end + + subject { worker.remaining_work_count } + + it { is_expected.to eq(3) } + end +end diff --git a/spec/workers/packages/cleanup_package_registry_worker_spec.rb b/spec/workers/packages/cleanup_package_registry_worker_spec.rb new file mode 100644 index 00000000000..e43864975f6 --- /dev/null +++ b/spec/workers/packages/cleanup_package_registry_worker_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::CleanupPackageRegistryWorker do + describe '#perform' do + let_it_be_with_reload(:package_files) { create_list(:package_file, 2, :pending_destruction) } + + let(:worker) { described_class.new } + + subject(:perform) { worker.perform } + + context 'with package files pending destruction' do + it_behaves_like 'an idempotent worker' + + it 'queues the cleanup job' do + expect(Packages::CleanupPackageFileWorker).to receive(:perform_with_capacity) + + perform + end + end + + context 'with no package files pending destruction' do + before do + ::Packages::PackageFile.update_all(status: :default) + end + + it_behaves_like 'an idempotent worker' + + it 'does not queue the cleanup job' do + expect(Packages::CleanupPackageFileWorker).not_to receive(:perform_with_capacity) + + perform + end + end + + describe 'counts logging' do + let_it_be(:processing_package_file) { create(:package_file, status: :processing) } + + it 'logs all the counts', :aggregate_failures do + expect(worker).to receive(:log_extra_metadata_on_done).with(:pending_destruction_package_files_count, 2) + expect(worker).to receive(:log_extra_metadata_on_done).with(:processing_package_files_count, 1) + expect(worker).to receive(:log_extra_metadata_on_done).with(:error_package_files_count, 0) + + perform + end + + context 'with load balancing enabled', :db_load_balancing do + it 'reads the count from the replica' do + expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original + + perform + end + end + end + end +end diff --git a/spec/workers/purge_dependency_proxy_cache_worker_spec.rb b/spec/workers/purge_dependency_proxy_cache_worker_spec.rb index b928104fb58..3de59670f8d 100644 --- a/spec/workers/purge_dependency_proxy_cache_worker_spec.rb +++ b/spec/workers/purge_dependency_proxy_cache_worker_spec.rb @@ -25,11 +25,11 @@ RSpec.describe PurgeDependencyProxyCacheWorker do include_examples 'an idempotent worker' do let(:job_args) { [user.id, group_id] } - it 'expires the blobs and returns ok', :aggregate_failures do + it 'marks the blobs as pending_destruction and returns ok', :aggregate_failures do subject - expect(blob).to be_expired - expect(manifest).to be_expired + expect(blob).to be_pending_destruction + expect(manifest).to be_pending_destruction end end end |