diff options
42 files changed, 499 insertions, 120 deletions
@@ -344,10 +344,10 @@ gem 'gitlab-license', '~> 2.3' # rubocop:todo Gemfile/MissingFeatureCategory gem 'rack-attack', '~> 6.7.0' # rubocop:todo Gemfile/MissingFeatureCategory # Sentry integration -gem 'sentry-raven', '~> 3.1', feature_category: :error_tracking -gem 'sentry-ruby', '~> 5.12', feature_category: :error_tracking -gem 'sentry-rails', '~> 5.12', feature_category: :error_tracking -gem 'sentry-sidekiq', '~> 5.12', feature_category: :error_tracking +gem 'sentry-raven', '~> 3.1' # rubocop:todo Gemfile/MissingFeatureCategory +gem 'sentry-ruby', '~> 5.8.0' # rubocop:todo Gemfile/MissingFeatureCategory +gem 'sentry-rails', '~> 5.8.0' # rubocop:todo Gemfile/MissingFeatureCategory +gem 'sentry-sidekiq', '~> 5.8.0' # rubocop:todo Gemfile/MissingFeatureCategory # PostgreSQL query parsing # diff --git a/Gemfile.checksum b/Gemfile.checksum index 59a1324ce23..3c79ae320bd 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -576,10 +576,10 @@ {"name":"seed-fu","version":"2.3.7","platform":"ruby","checksum":"f19673443e9af799b730e3d4eca6a89b39e5a36825015dffd00d02ea3365cf74"}, {"name":"selenium-webdriver","version":"4.14.0","platform":"ruby","checksum":"55726f81021d3f085ed9fcd318486b7ba90155598bb9e1fde7e4deeefe139d24"}, {"name":"semver_dialects","version":"1.2.1","platform":"ruby","checksum":"60a1f67659f79c51a667e8858ec9b089c1e4ce4f6d2a0f0b4ac101916946eb23"}, -{"name":"sentry-rails","version":"5.12.0","platform":"ruby","checksum":"7a1743c93aea9399646cd88c7ae5034fdbc9bac77106dda9b9840ae6dcbc0cc7"}, +{"name":"sentry-rails","version":"5.8.0","platform":"ruby","checksum":"c11b2d909de2c2bfda793c45f64180fd784d54c46886338b683ee3f8efa7731b"}, {"name":"sentry-raven","version":"3.1.2","platform":"ruby","checksum":"103d3b122958810d34898ce2e705bcf549ddb9d855a70ce9a3970ee2484f364a"}, -{"name":"sentry-ruby","version":"5.12.0","platform":"ruby","checksum":"2a8c161a9e5af6e8af251a778b5692fa3bfaf355a9cf83857eeef9f84e0e649a"}, -{"name":"sentry-sidekiq","version":"5.12.0","platform":"ruby","checksum":"c2e5b13cf67ae5baef8c246fba5d944ac90d6fcf9721594fb75a586124d082a4"}, +{"name":"sentry-ruby","version":"5.8.0","platform":"ruby","checksum":"caeb121433be379fb94e991a45265a287b13a9a9083e7264f539752369d37110"}, +{"name":"sentry-sidekiq","version":"5.8.0","platform":"ruby","checksum":"90d1123d16a9fc5fd99dbad190b766dd189eaf9e2baddad641f1334e1877c779"}, {"name":"set","version":"1.0.2","platform":"ruby","checksum":"02ffa4de1f2621495e05b72326040dd014d7abbcb02fea698bc600a389992c02"}, {"name":"sexp_processor","version":"4.17.0","platform":"ruby","checksum":"4daa4874ce1838cd801c65e66ed5d4f140024404a3de7482c36d4ef2604dff6f"}, {"name":"shellany","version":"0.0.1","platform":"ruby","checksum":"0e127a9132698766d7e752e82cdac8250b6adbd09e6c0a7fbbb6f61964fedee7"}, diff --git a/Gemfile.lock b/Gemfile.lock index 55d6fe0dd0e..6acbd4a1d4a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1484,15 +1484,15 @@ GEM pastel (~> 0.8.0) thor (~> 1.2.0) tty-command (~> 0.10.1) - sentry-rails (5.12.0) + sentry-rails (5.8.0) railties (>= 5.0) - sentry-ruby (~> 5.12.0) + sentry-ruby (~> 5.8.0) sentry-raven (3.1.2) faraday (>= 1.0) - sentry-ruby (5.12.0) + sentry-ruby (5.8.0) concurrent-ruby (~> 1.0, >= 1.0.2) - sentry-sidekiq (5.12.0) - sentry-ruby (~> 5.12.0) + sentry-sidekiq (5.8.0) + sentry-ruby (~> 5.8.0) sidekiq (>= 3.0) set (1.0.2) sexp_processor (4.17.0) @@ -2012,10 +2012,10 @@ DEPENDENCIES seed-fu (~> 2.3.7) selenium-webdriver (~> 4.14) semver_dialects (~> 1.2.1) - sentry-rails (~> 5.12) + sentry-rails (~> 5.8.0) sentry-raven (~> 3.1) - sentry-ruby (~> 5.12) - sentry-sidekiq (~> 5.12) + sentry-ruby (~> 5.8.0) + sentry-sidekiq (~> 5.8.0) shoulda-matchers (~> 5.1.0) sidekiq (~> 6.5.10) sidekiq-cron (~> 1.8.0) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/widget/app.vue b/app/assets/javascripts/vue_merge_request_widget/components/widget/app.vue index 8249dffcc27..08e803bffc9 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/widget/app.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/widget/app.vue @@ -9,6 +9,8 @@ export default { MrTerraformWidget: () => import('~/vue_merge_request_widget/extensions/terraform/index.vue'), MrCodeQualityWidget: () => import('~/vue_merge_request_widget/extensions/code_quality/index.vue'), + MrAccessibilityWidget: () => + import('~/vue_merge_request_widget/extensions/accessibility/index.vue'), }, props: { @@ -31,12 +33,17 @@ export default { return this.mr.codequalityReportsPath ? 'MrCodeQualityWidget' : undefined; }, + accessibilityWidget() { + return this.mr.accessibilityReportPath ? 'MrAccessibilityWidget' : undefined; + }, + widgets() { return [ this.codeQualityWidget, this.testReportWidget, this.terraformPlansWidget, 'MrSecurityWidget', + this.accessibilityWidget, ].filter((w) => w); }, }, diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js b/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.vue index 0fb5e13ad82..2ae16eef410 100644 --- a/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.vue @@ -1,24 +1,37 @@ +<script> import { uniqueId } from 'lodash'; import { __, n__, s__, sprintf } from '~/locale'; import axios from '~/lib/utils/axios_utils'; +import MrWidget from '~/vue_merge_request_widget/components/widget/widget.vue'; import { EXTENSION_ICONS } from '../../constants'; export default { name: 'WidgetAccessibility', - enablePolling: true, i18n: { loading: s__('Reports|Accessibility scanning results are being parsed'), error: s__('Reports|Accessibility scanning failed loading results'), }, - props: ['accessibilityReportPath'], + components: { + MrWidget, + }, + props: { + mr: { + type: Object, + required: true, + }, + }, + data() { + return { + collapsedData: {}, + content: [], + }; + }, computed: { statusIcon() { - return this.collapsedData.status === 'failed' + return this.collapsedData?.status === 'failed' ? EXTENSION_ICONS.warning : EXTENSION_ICONS.success; }, - }, - methods: { summary() { const numOfResults = this.collapsedData?.summary?.errored || 0; @@ -37,13 +50,20 @@ export default { false, ); - return numOfResults === 0 ? successText : warningText; + return numOfResults === 0 ? { title: successText } : { title: warningText }; }, shouldCollapse() { return this.collapsedData?.summary?.errored > 0; }, + }, + methods: { fetchCollapsedData() { - return axios.get(this.accessibilityReportPath); + return axios.get(this.mr.accessibilityReportPath).then((response) => { + this.collapsedData = response.data; + this.content = this.getContent(response.data); + + return response; + }); }, fetchFullData() { return Promise.resolve(this.prepareReports()); @@ -74,9 +94,7 @@ export default { formatMessage(message) { return sprintf(s__('AccessibilityReport|Message: %{message}'), { message }); }, - prepareReports() { - const { collapsedData } = this; - + getContent(collapsedData) { const newErrors = collapsedData.new_errors.map((error) => { return { header: __('New'), @@ -121,3 +139,16 @@ export default { }, }, }; +</script> +<template> + <mr-widget + :error-text="$options.i18n.error" + :status-icon-name="statusIcon" + :loading-text="$options.i18n.loading" + :widget-name="$options.name" + :summary="summary" + :content="content" + :is-collapsible="shouldCollapse" + :fetch-collapsed-data="fetchCollapsedData" + /> +</template> diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index 02d73cf9cbd..cc116b42f1e 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -1,9 +1,6 @@ <script> import { isEmpty, clamp } from 'lodash'; -import { - registerExtension, - registeredExtensions, -} from '~/vue_merge_request_widget/components/extensions'; +import { registeredExtensions } from '~/vue_merge_request_widget/components/extensions'; import SafeHtml from '~/vue_shared/directives/safe_html'; import MrWidgetApprovals from 'ee_else_ce/vue_merge_request_widget/components/approvals/approvals.vue'; import MRWidgetService from 'ee_else_ce/vue_merge_request_widget/services/mr_widget_service'; @@ -55,7 +52,6 @@ import eventHub from './event_hub'; import mergeRequestQueryVariablesMixin from './mixins/merge_request_query_variables'; import getStateQuery from './queries/get_state.query.graphql'; import getStateSubscription from './queries/get_state.subscription.graphql'; -import accessibilityExtension from './extensions/accessibility'; import ReportWidgetContainer from './components/report_widget_container.vue'; import MrWidgetReadyToMerge from './components/states/new_ready_to_merge.vue'; @@ -235,9 +231,6 @@ export default { false, ); }, - shouldShowAccessibilityReport() { - return Boolean(this.mr?.accessibilityReportPath); - }, formattedHumanAccess() { return (this.mr.humanAccess || '').toLowerCase(); }, @@ -268,11 +261,6 @@ export default { this.initPostMergeDeploymentsPolling(); } }, - shouldShowAccessibilityReport(newVal) { - if (newVal) { - this.registerAccessibilityExtension(); - } - }, }, mounted() { MRWidgetService.fetchInitialData() @@ -507,11 +495,6 @@ export default { dismissSuggestPipelines() { this.mr.isDismissedSuggestPipeline = true; }, - registerAccessibilityExtension() { - if (this.shouldShowAccessibilityReport) { - registerExtension(accessibilityExtension); - } - }, }, }; </script> diff --git a/app/assets/stylesheets/page_bundles/wiki.scss b/app/assets/stylesheets/page_bundles/wiki.scss index d87386f68ce..81e6b4c1191 100644 --- a/app/assets/stylesheets/page_bundles/wiki.scss +++ b/app/assets/stylesheets/page_bundles/wiki.scss @@ -148,7 +148,8 @@ margin: 0; } - ul.wiki-pages ul { + ul.wiki-pages ul, + ul.wiki-pages li:not(.wiki-directory){ padding-left: 20px; } diff --git a/app/graphql/mutations/merge_requests/accept.rb b/app/graphql/mutations/merge_requests/accept.rb index 220ebea22c7..604fdd49f45 100644 --- a/app/graphql/mutations/merge_requests/accept.rb +++ b/app/graphql/mutations/merge_requests/accept.rb @@ -9,6 +9,10 @@ module Mutations Accepts a merge request. When accepted, the source branch will be scheduled to merge into the target branch, either immediately if possible, or using one of the automatic merge strategies. + + [In GitLab 16.5](https://gitlab.com/gitlab-org/gitlab/-/issues/421510), the merging happens asynchronously. + This results in `mergeRequest` and `state` not updating after a mutation request, + because the merging may not have happened yet. DESC NOT_MERGEABLE = 'This branch cannot be merged' diff --git a/app/models/merge_request_diff_commit.rb b/app/models/merge_request_diff_commit.rb index 15fb356cb6d..790520c4123 100644 --- a/app/models/merge_request_diff_commit.rb +++ b/app/models/merge_request_diff_commit.rb @@ -6,13 +6,8 @@ class MergeRequestDiffCommit < ApplicationRecord include BulkInsertSafe include ShaAttribute include CachedCommit - include IgnorableColumns include FromUnion - ignore_column %i[author_name author_email committer_name committer_email], - remove_with: '14.6', - remove_after: '2021-11-22' - belongs_to :merge_request_diff # This relation is called `commit_author` and not `author`, as the project diff --git a/app/models/packages/npm/metadata_cache.rb b/app/models/packages/npm/metadata_cache.rb index 02efeda69cb..b6ab2a88a98 100644 --- a/app/models/packages/npm/metadata_cache.rb +++ b/app/models/packages/npm/metadata_cache.rb @@ -5,6 +5,9 @@ module Packages class MetadataCache < ApplicationRecord include FileStoreMounter include Packages::Downloadable + include Packages::Destructible + + enum status: { default: 0, processing: 1, error: 3 } belongs_to :project, inverse_of: :npm_metadata_caches @@ -18,6 +21,9 @@ module Packages before_validation :set_object_storage_key attr_readonly :object_storage_key + scope :stale, -> { where(project_id: nil) } + scope :pending_destruction, -> { stale.default } + def self.find_or_build(package_name:, project_id:) find_or_initialize_by( package_name: package_name, diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index aebce59a040..40a1a4392dd 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -5,6 +5,7 @@ class ProtectedBranch < ApplicationRecord include Gitlab::SQL::Pattern include FromUnion include EachBatch + include Presentable belongs_to :group, foreign_key: :namespace_id, touch: true, inverse_of: :protected_branches diff --git a/app/views/protected_branches/shared/_protected_branch.html.haml b/app/views/protected_branches/shared/_protected_branch.html.haml index 93c84e67d81..67c6e991a59 100644 --- a/app/views/protected_branches/shared/_protected_branch.html.haml +++ b/app/views/protected_branches/shared/_protected_branch.html.haml @@ -27,4 +27,14 @@ %span.has-tooltip{ data: { container: 'body' }, title: s_('ProtectedBranch|Inherited - This setting can be changed at the group level'), 'aria-hidden': 'true' } = sprite_icon 'lock' - else - = link_button_to s_('ProtectedBranch|Unprotect'), [protected_branch_entity, protected_branch, { update_section: 'js-protected-branches-settings' }], disabled: local_assigns[:disabled], aria: { label: s_('ProtectedBranch|Unprotect branch') }, data: { confirm: s_('ProtectedBranch|Branch will be writable for developers. Are you sure?'), confirm_btn_variant: 'danger' }, method: :delete, variant: :danger, category: :secondary, size: :small + .gl-relative + - if local_assigns[:protected_from_deletion] + %span.gl-absolute.gl-display-inline-block.gl-w-full.gl-h-full{ data: { container: 'body', toggle: 'popover', placement: local_assigns[:placemet], html: 'true', triggers: 'hover', content: local_assigns[:popover_content] } } + = render Pajamas::ButtonComponent.new(size: :small, + variant: :danger, + href: [protected_branch_entity, protected_branch, { update_section: 'js-protected-branches-settings' }], + method: :delete, + disabled: local_assigns[:protected_from_deletion], + button_options: { update_section: 'js-protected-branches-settings', aria: { label: s_('ProtectedBranch|Unprotect branch') }, data: { confirm: s_('ProtectedBranch|Branch will be writable for developers. Are you sure?'), confirm_btn_variant: 'danger' } }, + category: :secondary) do + = s_('ProtectedBranch|Unprotect') diff --git a/app/views/shared/wikis/_wiki_directory.html.haml b/app/views/shared/wikis/_wiki_directory.html.haml index cce81257691..8b0b6dbd8f7 100644 --- a/app/views/shared/wikis/_wiki_directory.html.haml +++ b/app/views/shared/wikis/_wiki_directory.html.haml @@ -1,12 +1,12 @@ - wiki_path = wiki_page_path(@wiki, wiki_directory) -%li{ class: active_when(params[:id] == wiki_directory.slug), data: { testid: 'wiki-directory-content' } } +%li{ class: ['wiki-directory', active_when(params[:id] == wiki_directory.slug)], data: { testid: 'wiki-directory-content' } } .gl-relative.gl-display-flex.gl-align-items-center.js-wiki-list-toggle.wiki-list{ data: { testid: 'wiki-list' } }< = sprite_icon('chevron-right', css_class: 'js-wiki-list-expand-button wiki-list-expand-button gl-mr-2 gl-cursor-pointer') = sprite_icon('chevron-down', css_class: 'js-wiki-list-collapse-button wiki-list-collapse-button gl-mr-2 gl-cursor-pointer') = render Pajamas::ButtonComponent.new(icon: 'plus', href: "#{wiki_path}/{new_page_title}", button_options: { class: 'wiki-list-create-child-button gl-bg-transparent! gl-hover-bg-gray-50! gl-focus-bg-gray-50! gl-absolute gl-top-half gl-translate-y-n50 gl-cursor-pointer gl-right-3' }) = link_to wiki_path, data: { testid: 'wiki-dir-page-link', qa_page_name: wiki_directory.title } do = wiki_directory.title - %ul + %ul.gl-pl-8 - wiki_directory.entries.each do |entry| = render partial: entry.to_partial_path, object: entry, locals: { context: context } diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 965053cf5f8..1ca099733e9 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1740,6 +1740,15 @@ :weight: 1 :idempotent: true :tags: [] +- :name: package_cleanup:packages_npm_cleanup_stale_metadata_cache + :worker_name: Packages::Npm::CleanupStaleMetadataCacheWorker + :feature_category: :package_registry + :has_external_dependencies: false + :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/packages/cleanup_package_registry_worker.rb b/app/workers/packages/cleanup_package_registry_worker.rb index 5f14102b5a1..5b2d8bacd62 100644 --- a/app/workers/packages/cleanup_package_registry_worker.rb +++ b/app/workers/packages/cleanup_package_registry_worker.rb @@ -13,6 +13,7 @@ module Packages def perform enqueue_package_file_cleanup_job if Packages::PackageFile.pending_destruction.exists? enqueue_cleanup_policy_jobs if Packages::Cleanup::Policy.runnable.exists? + enqueue_cleanup_stale_npm_metadata_cache_job if Packages::Npm::MetadataCache.pending_destruction.exists? log_counts end @@ -27,6 +28,10 @@ module Packages Packages::Cleanup::ExecutePolicyWorker.perform_with_capacity end + def enqueue_cleanup_stale_npm_metadata_cache_job + Packages::Npm::CleanupStaleMetadataCacheWorker.perform_with_capacity + end + def log_counts use_replica_if_available do pending_destruction_package_files_count = Packages::PackageFile.pending_destruction.count diff --git a/app/workers/packages/npm/cleanup_stale_metadata_cache_worker.rb b/app/workers/packages/npm/cleanup_stale_metadata_cache_worker.rb new file mode 100644 index 00000000000..158209c28fd --- /dev/null +++ b/app/workers/packages/npm/cleanup_stale_metadata_cache_worker.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Packages + module Npm + class CleanupStaleMetadataCacheWorker + include ApplicationWorker + include ::Packages::CleanupArtifactWorker + + MAX_CAPACITY = 2 + + data_consistency :sticky + + queue_namespace :package_cleanup + feature_category :package_registry + + deduplicate :until_executed + idempotent! + + def max_running_jobs + MAX_CAPACITY + end + + private + + def model + Packages::Npm::MetadataCache + end + + def log_metadata(npm_metadata_cache) + log_extra_metadata_on_done(:npm_metadata_cache_id, npm_metadata_cache.id) + end + + def log_cleanup_item(npm_metadata_cache) + logger.info( + structured_payload( + npm_metadata_cache_id: npm_metadata_cache.id + ) + ) + end + end + end +end diff --git a/db/migrate/20231019145202_add_status_to_packages_npm_metadata_caches.rb b/db/migrate/20231019145202_add_status_to_packages_npm_metadata_caches.rb new file mode 100644 index 00000000000..f3d910e9350 --- /dev/null +++ b/db/migrate/20231019145202_add_status_to_packages_npm_metadata_caches.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddStatusToPackagesNpmMetadataCaches < Gitlab::Database::Migration[2.1] + def change + add_column :packages_npm_metadata_caches, :status, :integer, default: 0, null: false, limit: 2 + end +end diff --git a/db/migrate/20231020181652_add_index_packages_npm_metadata_caches_on_id_and_project_id_and_status.rb b/db/migrate/20231020181652_add_index_packages_npm_metadata_caches_on_id_and_project_id_and_status.rb new file mode 100644 index 00000000000..6350ad935ca --- /dev/null +++ b/db/migrate/20231020181652_add_index_packages_npm_metadata_caches_on_id_and_project_id_and_status.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddIndexPackagesNpmMetadataCachesOnIdAndProjectIdAndStatus < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = 'idx_pkgs_npm_metadata_caches_on_id_and_project_id_and_status' + NPM_METADATA_CACHES_STATUS_DEFAULT = 0 + + def up + where = "project_id IS NULL AND status = #{NPM_METADATA_CACHES_STATUS_DEFAULT}" + + add_concurrent_index :packages_npm_metadata_caches, :id, name: INDEX_NAME, where: where + end + + def down + remove_concurrent_index_by_name :packages_npm_metadata_caches, name: INDEX_NAME + end +end diff --git a/db/schema_migrations/20231019145202 b/db/schema_migrations/20231019145202 new file mode 100644 index 00000000000..726093ee4dc --- /dev/null +++ b/db/schema_migrations/20231019145202 @@ -0,0 +1 @@ +c6a94dda004fccc8b3c8b5f59c7730a9243fe5d33a287997dae98748f3ad3bb4
\ No newline at end of file diff --git a/db/schema_migrations/20231020181652 b/db/schema_migrations/20231020181652 new file mode 100644 index 00000000000..3b0faf6040f --- /dev/null +++ b/db/schema_migrations/20231020181652 @@ -0,0 +1 @@ +ec632fbf61f89a45cb4f011117af10c26d847f822c2edcce637cbf18cb6a2b67
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3b4dd0300a1..d3faabf540f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -20211,6 +20211,7 @@ CREATE TABLE packages_npm_metadata_caches ( file text NOT NULL, package_name text NOT NULL, object_storage_key text NOT NULL, + status smallint DEFAULT 0 NOT NULL, CONSTRAINT check_57aa07a4b2 CHECK ((char_length(file) <= 255)), CONSTRAINT check_f97c15aa60 CHECK ((char_length(object_storage_key) <= 255)) ); @@ -31303,6 +31304,8 @@ CREATE UNIQUE INDEX idx_pkgs_dep_links_on_pkg_id_dependency_id_dependency_type O CREATE INDEX idx_pkgs_installable_package_files_on_package_id_id_file_name ON packages_package_files USING btree (package_id, id, file_name) WHERE (status = 0); +CREATE INDEX idx_pkgs_npm_metadata_caches_on_id_and_project_id_and_status ON packages_npm_metadata_caches USING btree (id) WHERE ((project_id IS NULL) AND (status = 0)); + CREATE INDEX idx_proj_feat_usg_on_jira_dvcs_cloud_last_sync_at_and_proj_id ON project_feature_usages USING btree (jira_dvcs_cloud_last_sync_at, project_id) WHERE (jira_dvcs_cloud_last_sync_at IS NOT NULL); CREATE INDEX idx_proj_feat_usg_on_jira_dvcs_server_last_sync_at_and_proj_id ON project_feature_usages USING btree (jira_dvcs_server_last_sync_at, project_id) WHERE (jira_dvcs_server_last_sync_at IS NOT NULL); diff --git a/doc/administration/packages/container_registry.md b/doc/administration/packages/container_registry.md index 84f86a21b2d..74dd71c19bf 100644 --- a/doc/administration/packages/container_registry.md +++ b/doc/administration/packages/container_registry.md @@ -637,11 +637,11 @@ you can pull from the Container Registry, but you cannot push. <!--- start_remove The following content will be removed on remove_date: '2023-10-22' --> WARNING: -The default configuration for the storage driver is scheduled to be [changed](https://gitlab.com/gitlab-org/container-registry/-/issues/854) in GitLab 16.0. The storage driver will use `/` as the default root directory. You can add `trimlegacyrootprefix: false` to your current configuration now to avoid any disruptions. For more information, see the [Container Registry configuration](https://gitlab.com/gitlab-org/container-registry/-/tree/master/docs-gitlab#azure-storage-driver) documentation. +The default configuration for the storage driver is scheduled to be [changed](https://gitlab.com/gitlab-org/container-registry/-/issues/854) in GitLab 16.0. The storage driver will use `/` as the default root directory. You can add `trimlegacyrootprefix: false` to your current configuration now to avoid any disruptions. For more information, see the [Container Registry configuration](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/upstream-differences.md#azure-storage-driver) documentation. <!--- end_remove --> When moving from an existing file system or another object storage provider to Azure Object Storage, you must configure the registry to use the standard root directory. -Configure it by setting [`trimlegacyrootprefix: true`](https://gitlab.com/gitlab-org/container-registry/-/blob/a3f64464c3ec1c5a599c0a2daa99ebcbc0100b9a/docs-gitlab/README.md#azure-storage-driver) in the Azure storage driver section of the registry configuration. +Configure it by setting [`trimlegacyrootprefix: true`](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/upstream-differences.md#azure-storage-driver) in the Azure storage driver section of the registry configuration. Without this configuration, the Azure storage driver uses `//` instead of `/` as the first section of the root path, rendering the migrated images inaccessible. ::Tabs @@ -1372,7 +1372,7 @@ By default, the container registry uses object storage to persist metadata related to container images. This method to store metadata limits how efficiently the data can be accessed, especially data spanning multiple images, such as when listing tags. By using a database to store this data, many new features are possible, including -[online garbage collection](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/db/online-garbage-collection.md) +[online garbage collection](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/gitlab/online-garbage-collection.md) which removes old data automatically with zero downtime. This database works in conjunction with the object storage already used by the registry, but does not replace object storage. diff --git a/doc/api/container_registry.md b/doc/api/container_registry.md index c44d800d39f..35b74965d2e 100644 --- a/doc/api/container_registry.md +++ b/doc/api/container_registry.md @@ -464,7 +464,7 @@ DELETE http(s)://${CI_REGISTRY}/v2/${CI_REGISTRY_IMAGE}/tags/reference/${CI_COMM ``` You can use the token retrieved with the predefined `CI_REGISTRY_USER` and `CI_REGISTRY_PASSWORD` variables to delete container image tags by reference on your GitLab instance. -The `tag_delete` [Container-Registry-Feature](https://gitlab.com/gitlab-org/container-registry/-/tree/v3.61.0-gitlab/docs-gitlab#api) must be enabled. +The `tag_delete` [Container-Registry-Feature](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/docker/v2/api.md#delete-tag) must be enabled. ```shell $ curl --request DELETE --header "Authorization: Bearer <token_from_above>" \ diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 6c22ffa5f22..965f9f52327 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -5136,6 +5136,10 @@ Accepts a merge request. When accepted, the source branch will be scheduled to merge into the target branch, either immediately if possible, or using one of the automatic merge strategies. +[In GitLab 16.5](https://gitlab.com/gitlab-org/gitlab/-/issues/421510), the merging happens asynchronously. +This results in `mergeRequest` and `state` not updating after a mutation request, +because the merging may not have happened yet. + Input type: `MergeRequestAcceptInput` #### Arguments diff --git a/doc/architecture/blueprints/container_registry_metadata_database/index.md b/doc/architecture/blueprints/container_registry_metadata_database/index.md index 5b73067128a..c9f7f1c0d27 100644 --- a/doc/architecture/blueprints/container_registry_metadata_database/index.md +++ b/doc/architecture/blueprints/container_registry_metadata_database/index.md @@ -30,7 +30,7 @@ graph LR R -- Write/read metadata --> B ``` -Client applications (for example, GitLab Rails and Docker CLI) interact with the Container Registry through its [HTTP API](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/api.md). The most common operations are pushing and pulling images to/from the registry, which require a series of HTTP requests in a specific order. The request flow for these operations is detailed in the [Request flow](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/push-pull-request-flow.md). +Client applications (for example, GitLab Rails and Docker CLI) interact with the Container Registry through its [HTTP API](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/gitlab/api.md). The most common operations are pushing and pulling images to/from the registry, which require a series of HTTP requests in a specific order. The request flow for these operations is detailed in the [Request flow](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/push-pull-request-flow.md). The registry supports multiple [storage backends](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/configuration.md#storage), including Google Cloud Storage (GCS) which is used for the GitLab.com registry. In the storage backend, images are stored as blobs, deduplicated, and shared across repositories. These are then linked (like a symlink) to each repository that relies on them, giving them access to the central storage location. @@ -69,7 +69,7 @@ Please refer to the [Docker documentation](https://docs.docker.com/registry/spec ##### Push and Pull -Push and pull commands are used to upload and download images, more precisely manifests and blobs. The push/pull flow is described in the [documentation](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/push-pull-request-flow.md). +Push and pull commands are used to upload and download images, more precisely manifests and blobs. The push/pull flow is described in the [documentation](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/push-pull-request-flow.md). #### GitLab Rails @@ -154,7 +154,7 @@ Following the GitLab [Go standards and style guidelines](../../../development/go The design and development of the registry database adhere to the GitLab [database guidelines](../../../development/database/index.md). Being a Go application, the required tooling to support the database will have to be developed, such as for running database migrations. -Running *online* and [*post deployment*](../../../development/database/post_deployment_migrations.md) migrations is already supported by the registry CLI, as described in the [documentation](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/database-migrations.md). +Running *online* and [*post deployment*](../../../development/database/post_deployment_migrations.md) migrations is already supported by the registry CLI, as described in the [documentation](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/database-migrations.md). #### Partitioning diff --git a/doc/architecture/blueprints/container_registry_metadata_database_self_managed_rollout/index.md b/doc/architecture/blueprints/container_registry_metadata_database_self_managed_rollout/index.md index 84a95e3e7c3..d91f2fdddbf 100644 --- a/doc/architecture/blueprints/container_registry_metadata_database_self_managed_rollout/index.md +++ b/doc/architecture/blueprints/container_registry_metadata_database_self_managed_rollout/index.md @@ -160,7 +160,7 @@ import which would lead to greater consistency across all storage driver impleme ### The Import Tool -The [import tool](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/database-import-tool.md) +The [import tool](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/database-import-tool.md) is a well-validated component of the Container Registry project that we have used from the beginning as a way to perform local testing. This tool is a thin wrapper over the core import functionality — the code which handles the import logic has diff --git a/doc/architecture/blueprints/google_artifact_registry_integration/index.md b/doc/architecture/blueprints/google_artifact_registry_integration/index.md index 4c2bfe95c5e..ef66ae33b2a 100644 --- a/doc/architecture/blueprints/google_artifact_registry_integration/index.md +++ b/doc/architecture/blueprints/google_artifact_registry_integration/index.md @@ -116,6 +116,6 @@ One alternative solution considered was to use the Docker/OCI API provided by GA - **Multiple Requests**: To retrieve all the required information about each image, multiple requests to different endpoints (listing tags, obtaining image manifests, and image configuration blobs) would have been necessary, leading to a `1+N` performance issue. -GitLab had previously faced significant challenges with the last two limitations, prompting the development of a custom [GitLab Container Registry API](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md) to address them. Additionally, GitLab decided to [deprecate support](../../../update/deprecations.md#use-of-third-party-container-registries-is-deprecated) for connecting to third-party container registries using the Docker/OCI API due to these same limitations and the increased cost of maintaining two solutions in parallel. As a result, there is an ongoing effort to replace the use of the Docker/OCI API endpoints with custom API endpoints for all container registry functionalities in GitLab. +GitLab had previously faced significant challenges with the last two limitations, prompting the development of a custom [GitLab Container Registry API](https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/gitlab/api.md) to address them. Additionally, GitLab decided to [deprecate support](../../../update/deprecations.md#use-of-third-party-container-registries-is-deprecated) for connecting to third-party container registries using the Docker/OCI API due to these same limitations and the increased cost of maintaining two solutions in parallel. As a result, there is an ongoing effort to replace the use of the Docker/OCI API endpoints with custom API endpoints for all container registry functionalities in GitLab. Considering these factors, the decision was made to build the GAR integration from scratch using the proprietary GAR API. This approach provides more flexibility and control over the integration and can serve as a foundation for future expansions, such as support for other GAR artifact formats. diff --git a/doc/architecture/blueprints/runner_admission_controller/index.md b/doc/architecture/blueprints/runner_admission_controller/index.md index 92c824527ec..21dc1d53303 100644 --- a/doc/architecture/blueprints/runner_admission_controller/index.md +++ b/doc/architecture/blueprints/runner_admission_controller/index.md @@ -1,7 +1,7 @@ --- status: proposed creation-date: "2023-03-07" -authors: [ "@ajwalker" ] +authors: [ "@ajwalker", "@johnwparent" ] coach: [ "@ayufan" ] approvers: [ "@DarrenEastman", "@engineering-manager" ] owning-stage: "~devops::<stage>" @@ -14,7 +14,7 @@ The GitLab `admission controller` (inspired by the [Kubernetes admission control An admission controller can be registered to the GitLab instance and receive a payload containing jobs to be created. Admission controllers can be _mutating_, _validating_, or both. -- When _mutating_, mutatable job information can be modified and sent back to the GitLab instance. Jobs can be modified to conform to organizational policy, security requirements, or have, for example, their tag list modified so that they're routed to specific runners. +- When _mutating_, mutable job information can be modified and sent back to the GitLab instance. Jobs can be modified to conform to organizational policy, security requirements, or have, for example, their tag list modified so that they're routed to specific runners. - When _validating_, a job can be denied execution. ## Motivation @@ -35,12 +35,12 @@ Before going further, it is helpful to level-set the current job handling mechan - On the request from a runner to the API for a job, the database is queried to verify that the job parameters matches that of the runner. In other words, when runners poll a GitLab instance for a job to execute they're assigned a job if it matches the specified criteria. - If the job matches the runner in question, then the GitLab instance connects the job to the runner and changes the job state to running. In other words, GitLab connects the `job` object with the `Runner` object. - A runner can be configured to run un-tagged jobs. Tags are the primary mechanism used today to enable customers to have some control of which Runners run certain types of jobs. -- So while runners are scoped to the instance, group, or project, there are no additional access control mechanisms today that can easily be expanded on to deny access to a runner based on a user or group identifier. +- So while runners are scoped to the instance, group, or project, there are no additional access control mechanisms today that can be expanded on to deny access to a runner based on a user or group identifier. -The current CI jobs queue logic is as follows. **Note - in the code ww still use the very old `build` naming construct, but we've migrated from `build` to `job` in the product and documentation. +The current CI jobs queue logic is as follows. **Note - in the code we still use the very old `build` naming construct, but we've migrated from `build` to `job` in the product and documentation. ```ruby -jobs = +jobs = if runner.instance_type? jobs_for_shared_runner elsif runner.group_type? @@ -96,22 +96,31 @@ Each runner has a tag such as `zone_a`, `zone_b`. In this scenario the customer 1. When a job is created the `project information` (`project_id`, `job_id`, `api_token`) will be used to query GitLab for specific details. 1. If the `user_id` matches then the admissions controller modifies the job tag list. `zone_a` is added to the tag list as the controller has detected that the user triggering the job should have their jobs run IN `zone_a`. +**Scenario 3**: Runner pool with specific tag scheme, user only has access to a specific subset + +Each runner has a tag identifier unique to that runner, e.g. `DiscoveryOne`, `tugNostromo`, `MVSeamus`, etc. Users have arbitrary access to these runners, however we don't want to fail a job on access denial, instead we want to prevent the job from being executed on runners to which the user does not have access. We also don't want to reduce the pool of runners the job can be run on. + +1. Configure an admissions controller to mutate jobs based on `user_id`. +1. When a job is created the `project information` (`project_id`, `job_id`, `api_token`) will be used to query GitLab for specific details. +1. The admission controller queries available runners with the `user_id` and collects all runners for which the job cannot be run. If this is _all_ runners, the admission controller rejects the job, which is dropped. No tags are modified, and a message is included indicating the reasoning. If there are runners for which the user has permissions, the admission controller filters the associated runners for which there are no permissions. + ### MVC #### Admission controller 1. A single admission controller can be registered at the instance level only. -1. The admission controller must respond within 30 seconds. -1. The admission controller will receive an array of individual jobs. These jobs may or may not be related to each other. The response must contain only responses to the jobs made as part of the request. +1. The admission controller must respond within 1 hr. +1. The admission controller will receive individual jobs. The response must contain only responses to that job. +1. The admission controller will recieve an API callback for rejection and acceptance, with the acceptance callback accepting mutation parameters. #### Job Lifecycle -1. The lifecycle of a job will be updated to include a new `validating` state. +1. The `preparing` job state will be expanded to include the validation process prerequisite. ```mermaid stateDiagram-v2 - created --> validating - state validating { + created --> preparing + state preparing { [*] --> accept [*] --> reject } @@ -127,10 +136,12 @@ Each runner has a tag such as `zone_a`, `zone_b`. In this scenario the customer executed --> created: retry ``` -1. When the state is `validating`, the mutating webhook payload is sent to the admission controller. -1. For jobs where the webhook times out (30 seconds) their status should be set as though the admission was denied. This should +1. When the state is `preparing`, the mutating webhook payload is sent to the admission controller asynchronously. This will be retried a number of times as needed. +1. The `preparing` state will wait for a response from the webhook or until timeout. +1. The UI should be updated with the current status of the job prerequisites and admission +1. For jobs where the webhook times out (1 hour) their status should be set as though the admission was denied with a timeout reasoning. This should be rare in typical circumstances. -1. Jobs with denied admission can be retried. Retried jobs will be resent to the admission controller along with any mutations that they received previously. +1. Jobs with denied admission can be retried. Retried jobs will be resent to the admission controller without tag mutations or runner filtering reset. 1. [`allow_failure`](../../../ci/yaml/index.md#allow_failure) should be updated to support jobs that fail on denied admissions, for example: ```yaml @@ -141,8 +152,8 @@ be rare in typical circumstances. on_denied_admission: true ``` -1. The UI should be updated to display the reason for any job mutations (if provided). -1. A table in the database should be created to store the mutations. Any changes that were made, like tags, should be persisted and attached to `ci_builds` with `acts_as_taggable :admission_tags`. +1. The UI should be updated to display the reason for any job mutations (if provided) or rejection. +1. Tag modifications applied by the Admission Controller should be persisted by the system with associated reasoning for any modifications, acceptances, or rejections #### Payload @@ -153,8 +164,10 @@ be rare in typical circumstances. 1. The response payload is comprised of individual job entries consisting of: - Job ID. - Admission state: `accepted` or `denied`. - - Mutations: Only `tags` is supported for now. The tags provided replaces the original tag list. + - Mutations: `additions` and `removals`. `additions` supplements the existing set of tags, `removals` removes tags from the current tag list - Reason: A controller can provide a reason for admission and mutation. + - Accepted Runners: runners to be considered for job matching, can be empty to match all runners + - Rejected Runners: runners that should not be considered for job matching, can be empty to match all runners ##### Example request @@ -170,7 +183,9 @@ be rare in typical circumstances. ... }, "tags": [ "docker", "windows" ] - }, + } +] +[ { "id": 245, "variables": { @@ -180,7 +195,9 @@ be rare in typical circumstances. ... }, "tags": [ "linux", "eu-west" ] - }, + } +] +[ { "id": 666, "variables": { @@ -202,20 +219,29 @@ be rare in typical circumstances. "id": 123, "admission": "accepted", "reason": "it's always-allow-day-wednesday" - }, + } +] +[ { "id": 245, "admission": "accepted", - "mutations": { - "tags": [ "linux", "us-west" ] + "tags": { + "add": [ "linux", "us-west" ], + "remove": [...] }, - "reason": "user is US employee: retagged region" - }, + "runners": { + "accepted_ids": ["822993167"], + "rejected_ids": ["822993168"] + }, + "reason": "user is US employee: retagged region; user only has uid on runner 822993167" + } +] +[ { "id": 666, "admission": "rejected", "reason": "you have no power here" - }, + } ] ``` @@ -229,13 +255,32 @@ be rare in typical circumstances. ### Implementation Details -1. _placeholder for steps required to code the admissions controller MVC_ +#### GitLab + +1. Expand `preparing` state to engage the validation process via the `prerequsite` interface. +1. Amend `preparing` state to indicate to user, via the UI and API, the status of job preparation with regard to the job prerequisites + 1. Should indicate status of each prerequisite resource for the job separately as they are asynchronous + 1. Should indicate overall prerequisite status +1. Introduce a 1 hr timeout to the entire `preparing` state +1. Add an `AdmissionValidation` prerequisite to the `preparing` status dependencies via `Gitlab::Ci::Build::Prerequisite::Factory` +1. Convert the Prerequisite factory and `preparing` status to operate asynchronously +1. Convert `PreparingBuildService` to operate asynchronously +1. `PreparingBuildService` transitions the job from preparing to failed or pending depending on success of validation. +1. AdmissionValidation performs a reasonable amount of retries when sending request +1. Add API endpoint for Webhook/Admission Controller response callback + 1. Accepts Parameters: + - Acceptance/Rejection + - Reason String + - Tag mutations (if accepted, otherwise ignored) + 1. Callback encodes one time auth token +1. Introduce new failure reasoning on validation rejection +1. Admission controller impacts on job should be persisted +1. Runner selection filtering per job as a function of the response from the Admission controller (mutating web hook) should be added ## Technical issues to resolve | issue | resolution| | ------ | ------ | -|We may have conflicting tag-sets as mutating controller can make it possible to define AND, OR and NONE logical definition of tags. This can get quite complex quickly. | | |Rule definition for the queue web hook| |What data to send to the admissions controller? Is it a subset or all of the [predefined variables](../../../ci/variables/predefined_variables.md)?| |Is the `queueing web hook` able to run at GitLab.com scale? On GitLab.com we would trigger millions of webhooks per second and the concern is that would overload Sidekiq or be used to abuse the system. diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index c3df15f1890..4236fe18c75 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -1731,6 +1731,7 @@ Some pages won't have a tier badge, because no obvious tier badge applies. For e - Tutorials. - Pages that compare features from different tiers. - Pages in the `/development` folder. These pages are automatically assigned a `Contribute` badge. +- Pages in the `/solutions` folder. These pages are automatically assigned a `Solutions` badge. ##### Administrator documentation tier badges diff --git a/doc/development/documentation/workflow.md b/doc/development/documentation/workflow.md index eb1ea28d3b8..fc0f4013104 100644 --- a/doc/development/documentation/workflow.md +++ b/doc/development/documentation/workflow.md @@ -65,6 +65,11 @@ Remember: - The Technical Writer can also help decide that documentation can be merged without Technical writer review, with the review to occur soon after merge. +## Pages with no tech writer review + +The documentation under `/doc/solutions` is created, maintained, copy edited, +and merged by the Solutions Architect team. + ## Do not use ChatGPT or AI-generated content for the docs GitLab documentation is distributed under the [CC BY-SA 4.0 license](https://creativecommons.org/licenses/by-sa/4.0/), which presupposes that GitLab owns the documentation. diff --git a/lib/container_registry/gitlab_api_client.rb b/lib/container_registry/gitlab_api_client.rb index dd912caaf99..9b6c37da847 100644 --- a/lib/container_registry/gitlab_api_client.rb +++ b/lib/container_registry/gitlab_api_client.rb @@ -103,7 +103,7 @@ module ContainerRegistry end end - # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#compliance-check + # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/gitlab/api.md#compliance-check def supports_gitlab_api? strong_memoize(:supports_gitlab_api) do registry_features = Gitlab::CurrentSettings.container_registry_features || [] @@ -116,19 +116,19 @@ module ContainerRegistry end end - # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#import-repository + # Deprecated. Will be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/409873. def pre_import_repository(path) response = start_import_for(path, pre: true) IMPORT_RESPONSES.fetch(response.status, :error) end - # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#import-repository + # Deprecated. Will be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/409873. def import_repository(path) response = start_import_for(path, pre: false) IMPORT_RESPONSES.fetch(response.status, :error) end - # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#cancel-repository-import + # Deprecated. Will be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/409873. def cancel_repository_import(path, force: false) response = with_import_token_faraday do |faraday_client| faraday_client.delete(import_url_for(path)) do |req| @@ -142,7 +142,7 @@ module ContainerRegistry { status: status, migration_state: actual_state } end - # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#get-repository-import-status + # Deprecated. Will be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/409873. def import_status(path) with_import_token_faraday do |faraday_client| response = faraday_client.get(import_url_for(path)) @@ -156,7 +156,7 @@ module ContainerRegistry end end - # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#get-repository-details + # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/gitlab/api.md#get-repository-details def repository_details(path, sizing: nil) with_token_faraday do |faraday_client| req = faraday_client.get("#{GITLAB_REPOSITORIES_PATH}/#{path}/") do |req| @@ -169,7 +169,7 @@ module ContainerRegistry end end - # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#list-repository-tags + # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/gitlab/api.md#list-repository-tags def tags(path, page_size: 100, last: nil, before: nil, name: nil, sort: nil) limited_page_size = [page_size, MAX_TAGS_PAGE_SIZE].min with_token_faraday do |faraday_client| @@ -202,7 +202,7 @@ module ContainerRegistry end end - # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#list-sub-repositories + # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/gitlab/api.md#list-sub-repositories def sub_repositories_with_tag(path, page_size: 100, last: nil) limited_page_size = [page_size, MAX_REPOSITORIES_PAGE_SIZE].min @@ -235,7 +235,7 @@ module ContainerRegistry # Given a path 'group/subgroup/project' and name 'newname', # with a successful rename, it will be 'group/subgroup/newname' - # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#rename-base-repository + # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs/spec/gitlab/api.md#rename-base-repository def rename_base_repository_path(path, name:, dry_run: false) with_token_faraday do |faraday_client| url = "#{GITLAB_REPOSITORIES_PATH}/#{path}/" diff --git a/qa/qa/page/project/job/show.rb b/qa/qa/page/project/job/show.rb index 1d253a62ceb..c1b2eca9fc2 100644 --- a/qa/qa/page/project/job/show.rb +++ b/qa/qa/page/project/job/show.rb @@ -68,12 +68,14 @@ module QA end end - def has_locked_artifact? - has_element?('artifacts-locked-message-content') + def has_locked_artifact?(wait: 240) + wait_until(reload: true, max_duration: wait, sleep_interval: 1) do + has_element?('artifacts-locked-message-content') + end end # Artifact unlock is async and depends on queue size on target env - def has_unlocked_artifact?(wait: 120) + def has_unlocked_artifact?(wait: 240) wait_until(reload: true, max_duration: wait, sleep_interval: 1) do has_element?('artifacts-unlocked-message-content') end diff --git a/spec/factories/packages/npm/metadata_cache.rb b/spec/factories/packages/npm/metadata_cache.rb index e76ddf3c983..4fe1930d03e 100644 --- a/spec/factories/packages/npm/metadata_cache.rb +++ b/spec/factories/packages/npm/metadata_cache.rb @@ -6,5 +6,15 @@ FactoryBot.define do sequence(:package_name) { |n| "@#{project.root_namespace.path}/package-#{n}" } file { fixture_file_upload('spec/fixtures/packages/npm/metadata.json') } size { 401.bytes } + + trait :processing do + status { 'processing' } + end + + trait :stale do + after(:create) do |entry| + entry.update_attribute(:project_id, nil) + end + end end end diff --git a/spec/frontend/vue_merge_request_widget/components/widget/app_spec.js b/spec/frontend/vue_merge_request_widget/components/widget/app_spec.js index 205824c3edd..1fc3b0c84ee 100644 --- a/spec/frontend/vue_merge_request_widget/components/widget/app_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/widget/app_spec.js @@ -5,6 +5,7 @@ import MrSecurityWidgetCE from '~/vue_merge_request_widget/extensions/security_r import MrTestReportWidget from '~/vue_merge_request_widget/extensions/test_report/index.vue'; import MrTerraformWidget from '~/vue_merge_request_widget/extensions/terraform/index.vue'; import MrCodeQualityWidget from '~/vue_merge_request_widget/extensions/code_quality/index.vue'; +import MrAccessibilityWidget from '~/vue_merge_request_widget/extensions/accessibility/index.vue'; describe('MR Widget App', () => { let wrapper; @@ -38,10 +39,11 @@ describe('MR Widget App', () => { }); describe.each` - widgetName | widget | endpoint - ${'testReportWidget'} | ${MrTestReportWidget} | ${'testResultsPath'} - ${'terraformPlansWidget'} | ${MrTerraformWidget} | ${'terraformReportsPath'} - ${'codeQualityWidget'} | ${MrCodeQualityWidget} | ${'codequalityReportsPath'} + widgetName | widget | endpoint + ${'testReportWidget'} | ${MrTestReportWidget} | ${'testResultsPath'} + ${'terraformPlansWidget'} | ${MrTerraformWidget} | ${'terraformReportsPath'} + ${'codeQualityWidget'} | ${MrCodeQualityWidget} | ${'codequalityReportsPath'} + ${'accessibilityWidget'} | ${MrAccessibilityWidget} | ${'accessibilityReportPath'} `('$widgetName', ({ widget, endpoint }) => { it(`is mounted when ${endpoint} is defined`, async () => { createComponent({ mr: { [endpoint]: `path/to/${endpoint}` } }); diff --git a/spec/frontend/vue_merge_request_widget/extentions/accessibility/index_spec.js b/spec/frontend/vue_merge_request_widget/extentions/accessibility/index_spec.js index 9b1e694d9c4..baeab1641d2 100644 --- a/spec/frontend/vue_merge_request_widget/extentions/accessibility/index_spec.js +++ b/spec/frontend/vue_merge_request_widget/extentions/accessibility/index_spec.js @@ -3,29 +3,25 @@ import { mountExtended } from 'helpers/vue_test_utils_helper'; import { trimText } from 'helpers/text_helper'; import waitForPromises from 'helpers/wait_for_promises'; import axios from '~/lib/utils/axios_utils'; -import extensionsContainer from '~/vue_merge_request_widget/components/extensions/container'; -import { registerExtension } from '~/vue_merge_request_widget/components/extensions'; -import accessibilityExtension from '~/vue_merge_request_widget/extensions/accessibility'; +import AccessibilityWidget from '~/vue_merge_request_widget/extensions/accessibility/index.vue'; import { HTTP_STATUS_INTERNAL_SERVER_ERROR, HTTP_STATUS_OK } from '~/lib/utils/http_status'; import { accessibilityReportResponseErrors, accessibilityReportResponseSuccess } from './mock_data'; -describe('Accessibility extension', () => { +describe('Accessibility widget', () => { let wrapper; let mock; - registerExtension(accessibilityExtension); - const endpoint = '/root/repo/-/merge_requests/4/accessibility_reports.json'; const mockApi = (statusCode, data) => { - mock.onGet(endpoint).reply(statusCode, data); + mock.onGet(endpoint).reply(statusCode, data, {}); }; const findToggleCollapsedButton = () => wrapper.findByTestId('toggle-button'); const findAllExtensionListItems = () => wrapper.findAllByTestId('extension-list-item'); const createComponent = () => { - wrapper = mountExtended(extensionsContainer, { + wrapper = mountExtended(AccessibilityWidget, { propsData: { mr: { accessibilityReportPath: endpoint, diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index 1ff7e48abfc..e1c0aafc3c3 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -797,20 +797,24 @@ RSpec.describe SearchHelper, feature_category: :global_search do allow(self).to receive(:current_user).and_return(:the_current_user) end - where(:input, :expected) do - '0' | false - '1' | true - 'yes' | true - 'no' | false - 'true' | true - 'false' | false - true | true - false | false + shared_context 'with inputs' do + where(:input, :expected) do + '0' | false + '1' | true + 'yes' | true + 'no' | false + 'true' | true + 'false' | false + true | true + false | false + end end describe 'for confidential' do let(:params) { { confidential: input } } + include_context 'with inputs' + with_them do it 'transforms param' do expect(::SearchService).to receive(:new).with(:the_current_user, { confidential: expected }) @@ -823,6 +827,8 @@ RSpec.describe SearchHelper, feature_category: :global_search do describe 'for include_archived' do let(:params) { { include_archived: input } } + include_context 'with inputs' + with_them do it 'transforms param' do expect(::SearchService).to receive(:new).with(:the_current_user, { include_archived: expected }) diff --git a/spec/migrations/20231019145202_add_status_to_packages_npm_metadata_caches_spec.rb b/spec/migrations/20231019145202_add_status_to_packages_npm_metadata_caches_spec.rb new file mode 100644 index 00000000000..eeeafbdc277 --- /dev/null +++ b/spec/migrations/20231019145202_add_status_to_packages_npm_metadata_caches_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe AddStatusToPackagesNpmMetadataCaches, feature_category: :package_registry do + let(:npm_metadata_caches) { table(:packages_npm_metadata_caches) } + + it 'correctly migrates up and down' do + reversible_migration do |migration| + migration.before -> { + expect(npm_metadata_caches.column_names).not_to include('status') + } + + migration.after -> { + npm_metadata_caches.reset_column_information + + expect(npm_metadata_caches.column_names).to include('status') + } + end + end +end diff --git a/spec/migrations/20231020181652_add_index_packages_npm_metadata_caches_on_id_and_project_id_and_status_spec.rb b/spec/migrations/20231020181652_add_index_packages_npm_metadata_caches_on_id_and_project_id_and_status_spec.rb new file mode 100644 index 00000000000..412ea33cb4e --- /dev/null +++ b/spec/migrations/20231020181652_add_index_packages_npm_metadata_caches_on_id_and_project_id_and_status_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe AddIndexPackagesNpmMetadataCachesOnIdAndProjectIdAndStatus, feature_category: :package_registry do + let(:index_name) { described_class::INDEX_NAME } + + it 'correctly migrates up and down' do + reversible_migration do |migration| + migration.before -> { + expect(ActiveRecord::Base.connection.indexes(:packages_npm_metadata_caches).map(&:name)) + .not_to include(index_name) + } + + migration.after -> { + # npm_metadata_caches.reset_column_information + + expect(ActiveRecord::Base.connection.indexes(:packages_npm_metadata_caches).map(&:name)) + .to include(index_name) + } + end + end +end diff --git a/spec/models/packages/npm/metadata_cache_spec.rb b/spec/models/packages/npm/metadata_cache_spec.rb index 94b41ab6a5e..3a6c87a4244 100644 --- a/spec/models/packages/npm/metadata_cache_spec.rb +++ b/spec/models/packages/npm/metadata_cache_spec.rb @@ -148,4 +148,40 @@ RSpec.describe Packages::Npm::MetadataCache, type: :model, feature_category: :pa end end end + + describe '.stale' do + let_it_be(:npm_metadata_cache) { create(:npm_metadata_cache) } + let_it_be(:npm_metadata_cache_stale) { create(:npm_metadata_cache, :stale) } + + subject { described_class.stale } + + it { is_expected.to contain_exactly(npm_metadata_cache_stale) } + end + + describe '.pending_destruction' do + let_it_be(:npm_metadata_cache) { create(:npm_metadata_cache) } + let_it_be(:npm_metadata_cache_stale_default) { create(:npm_metadata_cache, :stale) } + let_it_be(:npm_metadata_cache_stale_processing) { create(:npm_metadata_cache, :stale, :processing) } + + subject { described_class.pending_destruction } + + it { is_expected.to contain_exactly(npm_metadata_cache_stale_default) } + end + + describe '.next_pending_destruction' do + let_it_be(:npm_metadata_cache1) { create(:npm_metadata_cache, created_at: 1.month.ago, updated_at: 1.day.ago) } + let_it_be(:npm_metadata_cache2) { create(:npm_metadata_cache, created_at: 1.year.ago, updated_at: 1.year.ago) } + + let_it_be(:npm_metadata_cache3) do + create(:npm_metadata_cache, :stale, created_at: 2.years.ago, updated_at: 1.month.ago) + end + + let_it_be(:npm_metadata_cache4) do + create(:npm_metadata_cache, :stale, created_at: 3.years.ago, updated_at: 2.weeks.ago) + end + + it 'returns the oldest pending destruction item based on updated_at' do + expect(described_class.next_pending_destruction(order_by: :updated_at)).to eq(npm_metadata_cache3) + end + end end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index ebe7dbcbce1..4c2cff434a7 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -390,6 +390,7 @@ RSpec.describe 'Every Sidekiq worker', feature_category: :shared do 'Packages::Go::SyncPackagesWorker' => 3, 'Packages::MarkPackageFilesForDestructionWorker' => 3, 'Packages::Maven::Metadata::SyncWorker' => 3, + 'Packages::Npm::CleanupStaleMetadataCacheWorker' => 0, 'Packages::Nuget::ExtractionWorker' => 3, 'Packages::Rubygems::ExtractionWorker' => 3, 'PagesDomainSslRenewalWorker' => 3, diff --git a/spec/workers/packages/cleanup_package_registry_worker_spec.rb b/spec/workers/packages/cleanup_package_registry_worker_spec.rb index f70103070ef..f2787a92fbf 100644 --- a/spec/workers/packages/cleanup_package_registry_worker_spec.rb +++ b/spec/workers/packages/cleanup_package_registry_worker_spec.rb @@ -58,6 +58,28 @@ RSpec.describe Packages::CleanupPackageRegistryWorker, feature_category: :packag end end + context 'with npm metadata caches pending destruction' do + let_it_be(:npm_metadata_cache) { create(:npm_metadata_cache, :stale) } + + it_behaves_like 'an idempotent worker' + + it 'queues the cleanup job' do + expect(Packages::Npm::CleanupStaleMetadataCacheWorker).to receive(:perform_with_capacity) + + perform + end + end + + context 'with no npm metadata caches pending destruction' do + it_behaves_like 'an idempotent worker' + + it 'does not queue the cleanup job' do + expect(Packages::Npm::CleanupStaleMetadataCacheWorker).not_to receive(:perform_with_capacity) + + perform + end + end + describe 'counts logging' do let_it_be(:processing_package_file) { create(:package_file, status: :processing) } diff --git a/spec/workers/packages/npm/cleanup_stale_metadata_cache_worker_spec.rb b/spec/workers/packages/npm/cleanup_stale_metadata_cache_worker_spec.rb new file mode 100644 index 00000000000..390ed0ee453 --- /dev/null +++ b/spec/workers/packages/npm/cleanup_stale_metadata_cache_worker_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Npm::CleanupStaleMetadataCacheWorker, type: :worker, feature_category: :package_registry do + 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(:npm_metadata_cache1) { create(:npm_metadata_cache) } + let_it_be(:npm_metadata_cache2) { create(:npm_metadata_cache, :stale) } + + let_it_be(:npm_metadata_cache3) do + create(:npm_metadata_cache, :stale, updated_at: 1.year.ago, created_at: 1.year.ago) + end + + it 'deletes the oldest stale metadata cache based on id', :aggregate_failures do + expect(worker).to receive(:log_extra_metadata_on_done).with(:npm_metadata_cache_id, npm_metadata_cache2.id) + + expect { subject }.to change { Packages::Npm::MetadataCache.count }.by(-1) + expect { npm_metadata_cache2.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'with a stale metadata cache' do + let_it_be(:npm_metadata_cache) { create(:npm_metadata_cache, :stale) } + + context 'with an error during the destroy' do + before do + allow_next_found_instance_of(Packages::Npm::MetadataCache) do |metadata_cache| + allow(metadata_cache).to receive(:destroy!).and_raise('Error!') + end + end + + it 'handles the error' do + expect(Gitlab::ErrorTracking).to receive(:log_exception) + .with(instance_of(RuntimeError), class: described_class.name) + expect { subject }.to change { Packages::Npm::MetadataCache.error.count }.from(0).to(1) + expect(npm_metadata_cache.reload).to be_error + end + end + + context 'when trying to destroy a destroyed record' do + before do + allow_next_found_instance_of(Packages::Npm::MetadataCache) do |metadata_cache| + destroy_method = metadata_cache.method(:destroy!) + + allow(metadata_cache).to receive(:destroy!) do + destroy_method.call + + raise 'Error!' + end + end + end + + it 'handles the error' do + expect(Gitlab::ErrorTracking).to receive(:log_exception) + .with(instance_of(RuntimeError), class: described_class.name) + expect { subject }.not_to change { Packages::Npm::MetadataCache.count } + expect(npm_metadata_cache.reload).to be_error + end + end + end + end + + describe '#max_running_jobs' do + let(:capacity) { described_class::MAX_CAPACITY } + + subject { worker.max_running_jobs } + + it { is_expected.to eq(capacity) } + end +end |