diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-11-10 21:11:24 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-11-10 21:11:24 +0300 |
commit | 93b1f84ccb7c18fa6991fb950d54d4c9b6511b6c (patch) | |
tree | 18c1920dde2bb16f7fbb3d15206deca642a82892 /app | |
parent | a45283fcd27598eebd66a97041fa7d79d5a33aa5 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'app')
15 files changed, 327 insertions, 104 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 4331260db99..812548f6c16 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -146,6 +146,7 @@ export default { virtualScrollCurrentIndex: -1, subscribedToVirtualScrollingEvents: false, autoScrolled: false, + activeProject: undefined, }; }, apollo: { @@ -164,6 +165,7 @@ export default { const codeQualityBoolean = Boolean(this.endpointCodequality); const { codequalityReportsComparer, sastReport } = data?.project?.mergeRequest || {}; + this.activeProject = data?.project?.mergeRequest?.project; if ( (sastReport?.status === FINDINGS_STATUS_PARSED || !this.sastReportAvailable) && /* Checking for newErrors instead of a status indicator is a workaround that @@ -678,7 +680,7 @@ export default { <template> <div v-show="shouldShow"> - <findings-drawer :drawer="activeDrawer" @close="closeDrawer" /> + <findings-drawer :project="activeProject" :drawer="activeDrawer" @close="closeDrawer" /> <div v-if="isLoading || !isTreeLoaded" class="loading"><gl-loading-icon size="lg" /></div> <div v-else id="diffs" :class="{ active: shouldShow }" class="diffs tab-pane"> <compare-versions :diff-files-count-text="numTotalFiles" /> diff --git a/app/assets/javascripts/diffs/components/graphql/get_mr_codequality_and_security_reports.query.graphql b/app/assets/javascripts/diffs/components/graphql/get_mr_codequality_and_security_reports.query.graphql index 0def3a63b48..c02b041fee5 100644 --- a/app/assets/javascripts/diffs/components/graphql/get_mr_codequality_and_security_reports.query.graphql +++ b/app/assets/javascripts/diffs/components/graphql/get_mr_codequality_and_security_reports.query.graphql @@ -4,6 +4,11 @@ query getMRCodequalityAndSecurityReports($fullPath: ID!, $iid: String!) { mergeRequest(iid: $iid) { id title + project { + id + nameWithNamespace + fullPath + } hasSecurityReports codequalityReportsComparer { report { @@ -46,6 +51,12 @@ query getMRCodequalityAndSecurityReports($fullPath: ID!, $iid: String!) { status report { added { + identifiers { + externalId + externalType + name + url + } uuid title description diff --git a/app/assets/javascripts/diffs/components/shared/findings_drawer.vue b/app/assets/javascripts/diffs/components/shared/findings_drawer.vue index fddd455b17e..2c1a8305935 100644 --- a/app/assets/javascripts/diffs/components/shared/findings_drawer.vue +++ b/app/assets/javascripts/diffs/components/shared/findings_drawer.vue @@ -1,46 +1,56 @@ <script> -import { GlDrawer, GlIcon, GlLink } from '@gitlab/ui'; -import SafeHtml from '~/vue_shared/directives/safe_html'; -import { s__ } from '~/locale'; +import { GlBadge, GlDrawer, GlIcon, GlLink } from '@gitlab/ui'; +import { __, s__ } from '~/locale'; import { DRAWER_Z_INDEX } from '~/lib/utils/constants'; -import { SEVERITY_CLASSES, SEVERITY_ICONS } from '~/ci/reports/codequality_report/constants'; +import { getSeverity } from '~/ci/reports/utils'; import { getContentWrapperHeight } from '~/lib/utils/dom_utils'; +import DrawerItem from './findings_drawer_item.vue'; export const i18n = { - severity: s__('FindingsDrawer|Severity:'), - engine: s__('FindingsDrawer|Engine:'), - category: s__('FindingsDrawer|Category:'), - otherLocations: s__('FindingsDrawer|Other locations:'), + name: __('Name'), + description: __('Description'), + status: __('Status'), + sast: __('SAST'), + engine: __('Engine'), + identifiers: __('Identifiers'), + project: __('Project'), + file: __('File'), + tool: __('Tool'), + codeQualityFinding: s__('FindingsDrawer|Code Quality Finding'), + sastFinding: s__('FindingsDrawer|SAST Finding'), + codeQuality: s__('FindingsDrawer|Code Quality'), + detected: s__('FindingsDrawer|Detected in pipeline'), }; +export const codeQuality = 'codeQuality'; export default { i18n, - components: { GlDrawer, GlIcon, GlLink }, - directives: { - SafeHtml, - }, + codeQuality, + components: { GlBadge, GlDrawer, GlIcon, GlLink, DrawerItem }, props: { drawer: { type: Object, required: true, }, - }, - safeHtmlConfig: { - ALLOWED_TAGS: ['a', 'h1', 'h2', 'p'], - ALLOWED_ATTR: ['href', 'rel'], + project: { + type: Object, + required: false, + default: () => {}, + }, }, computed: { getDrawerHeaderHeight() { return getContentWrapperHeight(); }, + isCodeQuality() { + return this.drawer.scale === this.$options.codeQuality; + }, }, DRAWER_Z_INDEX, methods: { - severityClass(severity) { - return SEVERITY_CLASSES[severity] || SEVERITY_CLASSES.unknown; - }, - severityIcon(severity) { - return SEVERITY_ICONS[severity] || SEVERITY_ICONS.unknown; + getSeverity, + concatIdentifierName(name, index) { + return name + (index !== this.drawer.identifiers.length - 1 ? ', ' : ''); }, }, }; @@ -54,57 +64,82 @@ export default { @close="$emit('close')" > <template #title> - <h2 data-testid="findings-drawer-heading" class="gl-font-size-h2 gl-mt-0 gl-mb-0"> - {{ drawer.description }} + <h2 class="drawer-heading gl-font-base gl-mt-0 gl-mb-0"> + <gl-icon + :size="12" + :name="getSeverity(drawer).name" + :class="getSeverity(drawer).class" + class="inline-findings-severity-icon gl-vertical-align-baseline!" + /> + <span class="drawer-heading-severity">{{ drawer.severity }}</span> + {{ isCodeQuality ? $options.i18n.codeQualityFinding : $options.i18n.sastFinding }} </h2> </template> <template #default> <ul class="gl-list-style-none gl-border-b-initial gl-mb-0 gl-pb-0!"> - <li data-testid="findings-drawer-severity" class="gl-mb-4"> - <span class="gl-font-weight-bold">{{ $options.i18n.severity }}</span> - <gl-icon - data-testid="findings-drawer-severity-icon" - :size="12" - :name="severityIcon(drawer.severity)" - :class="severityClass(drawer.severity)" - class="inline-findings-severity-icon" - /> + <drawer-item v-if="drawer.title" :description="$options.i18n.name" :value="drawer.title" /> + + <drawer-item v-if="drawer.state" :description="$options.i18n.status"> + <template #value> + <gl-badge variant="warning" class="text-capitalize">{{ drawer.state }}</gl-badge> + </template> + </drawer-item> + + <drawer-item + v-if="drawer.description" + :description="$options.i18n.description" + :value="drawer.description" + /> + + <drawer-item + v-if="project && drawer.scale !== $options.codeQuality" + :description="$options.i18n.project" + > + <template #value> + <gl-link :href="`/${project.fullPath}`">{{ project.nameWithNamespace }}</gl-link> + </template> + </drawer-item> + + <drawer-item v-if="drawer.location || drawer.webUrl" :description="$options.i18n.file"> + <template #value> + <span v-if="drawer.webUrl && drawer.filePath && drawer.line"> + <gl-link :href="drawer.webUrl">{{ drawer.filePath }}:{{ drawer.line }}</gl-link> + </span> + <span v-else-if="drawer.location"> + {{ drawer.location.file }}:{{ drawer.location.startLine }} + </span> + </template> + </drawer-item> + + <drawer-item + v-if="drawer.identifiers && drawer.identifiers.length" + :description="$options.i18n.identifiers" + > + <template #value> + <span v-for="(identifier, index) in drawer.identifiers" :key="identifier.externalId"> + <gl-link v-if="identifier.url" :href="identifier.url"> + {{ concatIdentifierName(identifier.name, index) }} + </gl-link> + <span v-else> + {{ concatIdentifierName(identifier.name, index) }} + </span> + </span> + </template> + </drawer-item> + + <drawer-item + v-if="drawer.scale" + :description="$options.i18n.tool" + :value="isCodeQuality ? $options.i18n.codeQuality : $options.i18n.sast" + /> - {{ drawer.severity }} - </li> - <li data-testid="findings-drawer-engine" class="gl-mb-4"> - <span class="gl-font-weight-bold">{{ $options.i18n.engine }}</span> - {{ drawer.engineName }} - </li> - <li data-testid="findings-drawer-category" class="gl-mb-4"> - <span class="gl-font-weight-bold">{{ $options.i18n.category }}</span> - {{ drawer.categories ? drawer.categories[0] : '' }} - </li> - <li data-testid="findings-drawer-other-locations" class="gl-mb-4"> - <span class="gl-font-weight-bold gl-mb-3 gl-display-block">{{ - $options.i18n.otherLocations - }}</span> - <ul class="gl-pl-6"> - <li - v-for="otherLocation in drawer.otherLocations" - :key="otherLocation.path" - class="gl-mb-1" - > - <gl-link - data-testid="findings-drawer-other-locations-link" - :href="otherLocation.href" - >{{ otherLocation.path }}</gl-link - > - </li> - </ul> - </li> + <drawer-item + v-if="drawer.engineName" + :description="$options.i18n.engine" + :value="drawer.engineName" + /> </ul> - <span - v-safe-html:[$options.safeHtmlConfig]="drawer.content ? drawer.content.body : ''" - data-testid="findings-drawer-body" - class="drawer-body gl-display-block gl-px-3 gl-py-0!" - ></span> </template> </gl-drawer> </template> diff --git a/app/assets/javascripts/diffs/components/shared/findings_drawer_item.vue b/app/assets/javascripts/diffs/components/shared/findings_drawer_item.vue new file mode 100644 index 00000000000..f488e8e3bb1 --- /dev/null +++ b/app/assets/javascripts/diffs/components/shared/findings_drawer_item.vue @@ -0,0 +1,30 @@ +<script> +export default { + props: { + description: { + type: String, + required: false, + default: '', + }, + value: { + type: String, + required: false, + default: '', + }, + }, +}; +</script> +<template> + <li class="gl-mb-4"> + <p class="gl-line-height-20"> + <span + data-testid="findings-drawer-item-description" + class="gl-font-weight-bold gl-display-block gl-mb-1" + >{{ description }}</span + > + <slot name="value"> + <span data-testid="findings-drawer-item-value-prop">{{ value }}</span> + </slot> + </p> + </li> +</template> diff --git a/app/assets/javascripts/diffs/utils/sort_findings_by_file.js b/app/assets/javascripts/diffs/utils/sort_findings_by_file.js index 3a285e80ace..3cf6dc169e4 100644 --- a/app/assets/javascripts/diffs/utils/sort_findings_by_file.js +++ b/app/assets/javascripts/diffs/utils/sort_findings_by_file.js @@ -1,10 +1,17 @@ export function sortFindingsByFile(newErrors = []) { const files = {}; - newErrors.forEach(({ filePath, line, description, severity }) => { + newErrors.forEach(({ line, description, severity, filePath, webUrl, engineName }) => { if (!files[filePath]) { files[filePath] = []; } - files[filePath].push({ line, description, severity: severity.toLowerCase() }); + files[filePath].push({ + line, + description, + severity: severity.toLowerCase(), + filePath, + webUrl, + engineName, + }); }); const sortedFiles = Object.keys(files) diff --git a/app/assets/javascripts/vue_shared/components/diff_stats_dropdown.vue b/app/assets/javascripts/vue_shared/components/diff_stats_dropdown.vue index 70daac311c7..55767c5f4bc 100644 --- a/app/assets/javascripts/vue_shared/components/diff_stats_dropdown.vue +++ b/app/assets/javascripts/vue_shared/components/diff_stats_dropdown.vue @@ -118,9 +118,9 @@ export default { </span> </template> <template #list-item="{ item }"> - <div class="gl-display-flex gl-gap-3 gl-align-items-center"> - <gl-icon :name="item.icon" :class="item.iconColor" /> - <div class="gl-flex-grow-1"> + <div class="gl-display-flex gl-gap-3 gl-align-items-center gl-overflow-hidden"> + <gl-icon :name="item.icon" :class="item.iconColor" class="gl-flex-shrink-0" /> + <div class="gl-flex-grow-1 gl-overflow-hidden"> <div class="gl-display-flex"> <span class="gl-font-weight-bold gl-mr-3 gl-flex-grow-1" diff --git a/app/finders/ci/catalog/resources/versions_finder.rb b/app/finders/ci/catalog/resources/versions_finder.rb new file mode 100644 index 00000000000..b37d4f0377a --- /dev/null +++ b/app/finders/ci/catalog/resources/versions_finder.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Ci + module Catalog + module Resources + class VersionsFinder + include Gitlab::Utils::StrongMemoize + + def initialize(catalog_resources, current_user, params = {}) + # The catalog resources should already have their project association preloaded + @catalog_resources = Array.wrap(catalog_resources) + @current_user = current_user + @params = params + end + + def execute + return Ci::Catalog::Resources::Version.none if authorized_catalog_resources.empty? + + versions = params[:latest] ? get_latest_versions : get_versions + versions = versions.preloaded + sort(versions) + end + + private + + DEFAULT_SORT = :released_at_desc + + attr_reader :catalog_resources, :current_user, :params + + def get_versions + Ci::Catalog::Resources::Version.for_catalog_resources(authorized_catalog_resources) + end + + def get_latest_versions + Ci::Catalog::Resources::Version.latest_for_catalog_resources(authorized_catalog_resources) + end + + def authorized_catalog_resources + # Preload project authorizations to avoid N+1 queries + projects = catalog_resources.map(&:project) + ActiveRecord::Associations::Preloader.new(records: projects, associations: :project_feature).call + Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects, current_user).execute + + catalog_resources.select { |resource| authorized?(resource.project) } + end + strong_memoize_attr :authorized_catalog_resources + + def sort(versions) + versions.order_by(params[:sort] || DEFAULT_SORT) + end + + def authorized?(project) + Ability.allowed?(current_user, :read_release, project) + end + end + end + end +end diff --git a/app/models/ci/catalog/resource.rb b/app/models/ci/catalog/resource.rb index 6ade90809b0..f947c5158cf 100644 --- a/app/models/ci/catalog/resource.rb +++ b/app/models/ci/catalog/resource.rb @@ -15,7 +15,8 @@ module Ci belongs_to :project has_many :components, class_name: 'Ci::Catalog::Resources::Component', foreign_key: :catalog_resource_id, inverse_of: :catalog_resource - has_many :versions, class_name: 'Ci::Catalog::Resources::Version', inverse_of: :catalog_resource + has_many :versions, class_name: 'Ci::Catalog::Resources::Version', foreign_key: :catalog_resource_id, + inverse_of: :catalog_resource scope :for_projects, ->(project_ids) { where(project_id: project_ids) } scope :search, ->(query) { fuzzy_search(query, [:name, :description], use_minimum_char_limit: false) } @@ -33,14 +34,6 @@ module Ci before_create :sync_with_project - def versions - project.releases.order_released_desc - end - - def latest_version - project.releases.latest - end - def unpublish! update!(state: :draft) end diff --git a/app/models/ci/catalog/resources/version.rb b/app/models/ci/catalog/resources/version.rb index fae6d9846f9..bd0ebc77a6d 100644 --- a/app/models/ci/catalog/resources/version.rb +++ b/app/models/ci/catalog/resources/version.rb @@ -16,6 +16,100 @@ module Ci has_many :components, class_name: 'Ci::Catalog::Resources::Component', inverse_of: :version validates :release, :catalog_resource, :project, presence: true + + scope :for_catalog_resources, ->(catalog_resources) { where(catalog_resource_id: catalog_resources) } + scope :preloaded, -> { includes(:catalog_resource, project: [:route, { namespace: :route }], release: :author) } + + scope :order_by_created_at_asc, -> { reorder(created_at: :asc) } + scope :order_by_created_at_desc, -> { reorder(created_at: :desc) } + # After we denormalize the `released_at` column, we won't need to use `joins(:release)` and keyset_order_* + scope :order_by_released_at_asc, -> { joins(:release).keyset_order_by_released_at_asc } + scope :order_by_released_at_desc, -> { joins(:release).keyset_order_by_released_at_desc } + + delegate :name, :description, :tag, :sha, :released_at, :author_id, to: :release + + class << self + # In the future, we should support semantic versioning. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/427286 + def latest + order_by_released_at_desc.first + end + + # This query uses LATERAL JOIN to find the latest version for each catalog resource. To avoid + # joining the `catalog_resources` table, we build an in-memory table using the resource ids. + # Example: + # SELECT ... + # FROM (VALUES (CATALOG_RESOURCE_ID_1),(CATALOG_RESOURCE_ID_2)) catalog_resources (id) + # INNER JOIN LATERAL (...) + def latest_for_catalog_resources(catalog_resources) + return none if catalog_resources.empty? + + catalog_resources_table = Ci::Catalog::Resource.arel_table + catalog_resources_id_list = catalog_resources.map { |resource| "(#{resource.id})" }.join(',') + + # We need to use an alias for the `releases` table here so that it does not + # conflict with `joins(:release)` in the `order_by_released_at_*` scope. + join_query = Ci::Catalog::Resources::Version + .where(catalog_resources_table[:id].eq(arel_table[:catalog_resource_id])) + .joins("INNER JOIN releases AS rel ON rel.id = #{table_name}.release_id") + .order(Arel.sql('rel.released_at DESC')) + .limit(1) + + Ci::Catalog::Resources::Version + .from("(VALUES #{catalog_resources_id_list}) #{catalog_resources_table.name} (id)") + .joins("INNER JOIN LATERAL (#{join_query.to_sql}) #{table_name} ON TRUE") + end + + def keyset_order_by_released_at_asc + keyset_order = Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: :released_at, + column_expression: Release.arel_table[:released_at], + order_expression: Release.arel_table[:released_at].asc, + nullable: :not_nullable, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: :id, + order_expression: Release.arel_table[:id].asc, + nullable: :not_nullable, + distinct: true + ) + ]) + + reorder(keyset_order) + end + + def keyset_order_by_released_at_desc + keyset_order = Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: :released_at, + column_expression: Release.arel_table[:released_at], + order_expression: Release.arel_table[:released_at].desc, + nullable: :not_nullable, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: :id, + order_expression: Release.arel_table[:id].desc, + nullable: :not_nullable, + distinct: true + ) + ]) + + reorder(keyset_order) + end + + def order_by(order) + case order.to_s + when 'created_asc' then order_by_created_at_asc + when 'created_desc' then order_by_created_at_desc + when 'released_at_asc' then order_by_released_at_asc + else + order_by_released_at_desc + end + end + end end end end diff --git a/app/models/concerns/enums/package_metadata.rb b/app/models/concerns/enums/package_metadata.rb index cba6cd2db2e..352eb41829b 100644 --- a/app/models/concerns/enums/package_metadata.rb +++ b/app/models/concerns/enums/package_metadata.rb @@ -14,7 +14,7 @@ module Enums apk: 9, rpm: 10, deb: 11, - cbl_mariner: 12, + 'cbl-mariner': 12, wolfi: 13 }.with_indifferent_access.freeze diff --git a/app/models/concerns/enums/sbom.rb b/app/models/concerns/enums/sbom.rb index 64e0a7653d6..af8e37b4248 100644 --- a/app/models/concerns/enums/sbom.rb +++ b/app/models/concerns/enums/sbom.rb @@ -18,7 +18,7 @@ module Enums apk: 9, rpm: 10, deb: 11, - cbl_mariner: 12, + 'cbl-mariner': 12, wolfi: 13 }.with_indifferent_access.freeze diff --git a/app/models/concerns/repository_storage_movable.rb b/app/models/concerns/repository_storage_movable.rb index 89bdef1ccc6..b1dbebff4fb 100644 --- a/app/models/concerns/repository_storage_movable.rb +++ b/app/models/concerns/repository_storage_movable.rb @@ -46,6 +46,8 @@ module RepositoryStorageMovable transition replicated: :cleanup_failed end + # An after_transition can't affect the success of the transition. + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45160#note_431071664 around_transition initial: :scheduled do |storage_move, block| block.call @@ -64,13 +66,9 @@ module RepositoryStorageMovable true end - before_transition started: :replicated do |storage_move| + after_transition started: :replicated do |storage_move| storage_move.container.set_repository_writable! - storage_move.update_repository_storage(storage_move.destination_storage_name) - end - - after_transition started: :replicated do |storage_move| # We have several scripts in place that replicate some statistics information # to other databases. Some of them depend on the updated_at column # to identify the models they need to extract. @@ -86,6 +84,13 @@ module RepositoryStorageMovable storage_move.container.set_repository_writable! end + # This callback ensures the repository is set to writable in the event of + # a connection error during the :started -> :replicated transition + # https://gitlab.com/gitlab-org/gitlab/-/issues/427254#note_1636072125 + before_transition replicated: :cleanup_failed do |storage_move| + storage_move.container.set_repository_writable! + end + state :initial, value: 1 state :scheduled, value: 2 state :started, value: 3 @@ -96,15 +101,6 @@ module RepositoryStorageMovable end end - # Projects, snippets, and group wikis has different db structure. In projects, - # we need to update some columns in this step, but we don't with the other resources. - # - # Therefore, we create this No-op method for snippets and wikis and let project - # overwrite it in their implementation. - def update_repository_storage(new_storage) - # No-op - end - def schedule_repository_storage_update_worker raise NotImplementedError end diff --git a/app/models/projects/repository_storage_move.rb b/app/models/projects/repository_storage_move.rb index f4411e0b4fd..e2c6d1853a9 100644 --- a/app/models/projects/repository_storage_move.rb +++ b/app/models/projects/repository_storage_move.rb @@ -14,11 +14,6 @@ module Projects alias_attribute :project, :container scope :with_projects, -> { includes(container: :route) } - override :update_repository_storage - def update_repository_storage(new_storage) - container.update_column(:repository_storage, new_storage) - end - override :schedule_repository_storage_update_worker def schedule_repository_storage_update_worker Projects::UpdateRepositoryStorageWorker.perform_async( diff --git a/app/services/concerns/update_repository_storage_methods.rb b/app/services/concerns/update_repository_storage_methods.rb index 50963cc58b2..aff36d6943e 100644 --- a/app/services/concerns/update_repository_storage_methods.rb +++ b/app/services/concerns/update_repository_storage_methods.rb @@ -32,9 +32,9 @@ module UpdateRepositoryStorageMethods end end - repository_storage_move.transaction do - repository_storage_move.finish_replication! + repository_storage_move.finish_replication! + repository_storage_move.transaction do track_repository(destination_storage_name) end diff --git a/app/services/projects/update_repository_storage_service.rb b/app/services/projects/update_repository_storage_service.rb index 85fb1890fcd..a9f6afb26c9 100644 --- a/app/services/projects/update_repository_storage_service.rb +++ b/app/services/projects/update_repository_storage_service.rb @@ -8,7 +8,9 @@ module Projects private - def track_repository(_destination_storage_name) + def track_repository(destination_storage_name) + project.update!(repository_storage: destination_storage_name) + # Connect project to pool repository from the new shard project.swap_pool_repository! |