diff options
36 files changed, 1038 insertions, 331 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! diff --git a/doc/architecture/blueprints/cloud_connector/decisions/001_lb_entry_point.md b/doc/architecture/blueprints/cloud_connector/decisions/001_lb_entry_point.md new file mode 100644 index 00000000000..d49b702be94 --- /dev/null +++ b/doc/architecture/blueprints/cloud_connector/decisions/001_lb_entry_point.md @@ -0,0 +1,52 @@ +--- +owning-stage: "~devops::data stores" +description: 'Cloud Connector ADR 001: Use load balancer as single entry point' +--- + +# Cloud Connector ADR 001: Load balancer as single entry point + +## Context + +The original iteration of the blueprint suggested to stand up a dedicated Cloud Connector edge service, +through which all traffic that uses features under the Cloud Connector umbrella would pass. + +The primary reasons for why we wanted this to be a dedicated service were to: + +1. **Provide a single entry point for customers.** We identified the ability for any GitLab instance + around the world to consume Cloud Connector features through a single endpoint such as + `cloud.gitlab.com` as a must-have property. +1. **Have the ability to execute custom logic.** There was a desire from product to create a space where we can + run cross-cutting business logic such as application-level rate limiting, which is hard or impossible to + do using a traditional load balancer such as HAProxy. + +## Decision + +We decided to take a smaller incremental step toward having a "smart router" by focusing on +the ability to provide a single endpoint through which Cloud Connector traffic enters our +infrastructure. This can be accomplished using simpler means than deploying dedicated services, specifically +by pulling in a load balancing layer listening at `cloud.gitlab.com` that can also perform simple routing +tasks to forward traffic into feature backends. + +Our reasons for this decision were: + +1. **Unclear requirements for custom logic to run.** We are still exploring how and to what extent we would + apply rate limiting logic at the Cloud Connector level. This is being explored in + [issue 429592](https://gitlab.com/gitlab-org/gitlab/-/issues/429592). Because we need to have a single + entry point by January, and because we think we will not be ready by then to implement such logic at the + Cloud Connector level, a web service is not required yet. +1. **New use cases found that are not suitable to run through a dedicated service.** We started to work with + the Observability group to see how we can bring the GitLab Observability Backend (GOB) to Cloud Connector + customers in [MR 131577](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131577). + In this discussion it became clear that due to the large amounts of traffic and data volume passing + through GOB each day, putting another service in front of this stack does not provide a sensible + risk/benefit trade-off. Instead, we will probably split traffic and make Cloud Connector components + available through other means for special cases like these (for example, through a Cloud Connector library). + +We are exploring several options for load-balancing this new endpoint in [issue 429818](https://gitlab.com/gitlab-org/gitlab/-/issues/429818) +and are working with the `Infrastructure:Foundations` team to deploy this in [issue 24711](https://gitlab.com/gitlab-com/gl-infra/reliability/-/issues/24711). + +## Consequences + +We have not yet discarded the plan to build a smart router eventually, either as a service or +through other means, but have delayed this decision in face of uncertainty at both a product +and technical level. We will reassess how to proceed in Q1 2024. diff --git a/doc/architecture/blueprints/cloud_connector/index.md b/doc/architecture/blueprints/cloud_connector/index.md index 840e17a438a..9aef8bc7a98 100644 --- a/doc/architecture/blueprints/cloud_connector/index.md +++ b/doc/architecture/blueprints/cloud_connector/index.md @@ -68,7 +68,7 @@ Introducing a dedicated edge service for Cloud Connector serves the following go we do not currently support. - **Independently scalable.** For reasons of fault tolerance and scalability, it is beneficial to have all SM traffic go through a separate service. For example, if an excess of unexpected requests arrive from SM instances due to a bug - in a milestone release, this traffic could be absorbed at the CC gateway level without cascading downstream, thus leaving + in a milestone release, this traffic could be absorbed at the CC gateway level without cascading further, thus leaving SaaS users unaffected. ### Non-goals @@ -82,6 +82,10 @@ Introducing a dedicated edge service for Cloud Connector serves the following go other systems using public key cryptographic checks. We may move some of the code around that currently implements this, however. +## Decisions + +- [ADR-001: Use load balancer as single entry point](decisions/001_lb_entry_point.md) + ## Proposal We propose to make two major changes to the current architecture: @@ -133,7 +137,7 @@ The new service would be made available at `cloud.gitlab.com` and act as a "smar It will have the following responsibilities: 1. **Request handling.** The service will make decisions about whether a particular request is handled - in the service itself or forwarded to a downstream service. For example, a request to `/ai/code_suggestions/completions` + in the service itself or forwarded to other backends. For example, a request to `/ai/code_suggestions/completions` could be handled by forwarding this request to an appropriate endpoint in the AI gateway unchanged, while a request to `/-/metrics` could be handled by the service itself. As mentioned in [non-goals](#non-goals), the latter would not include domain logic as it pertains to an end user feature, but rather cross-cutting logic such as telemetry, or @@ -141,14 +145,14 @@ It will have the following responsibilities: When handling requests, the service should be unopinionated about which protocol is used, to the extent possible. Reasons for injecting custom logic could be setting additional HTTP header fields. A design principle should be - to not require CC service deployments if a downstream service merely changes request payload or endpoint definitions. However, + to not require CC service deployments if a backend service merely changes request payload or endpoint definitions. However, supporting more protocols on top of HTTP may require adding support in the CC service itself. 1. **Authentication/authorization.** The service will be the first point of contact for authenticating clients and verifying they are authorized to use a particular CC feature. This will include fetching and caching public keys served from GitLab SaaS and CustomersDot to decode JWT access tokens sent by GitLab instances, including matching token scopes to feature endpoints to ensure an instance is eligible to consume this feature. This functionality will largely be lifted out of the AI gateway where it currently lives. To maintain a ZeroTrust environment, the service will implement a more lightweight auth/z protocol - with internal services downstream that merely performs general authenticity checks but forgoes billing and permission + with internal backends that merely performs general authenticity checks but forgoes billing and permission related scoping checks. How this protocol will look like is to be decided, and might be further explored in [Discussion: Standardized Authentication and Authorization between internal services and GitLab Rails](https://gitlab.com/gitlab-org/gitlab/-/issues/421983). 1. **Organization-level rate limits.** It is to be decided if this is needed, but there could be value in having application-level rate limits diff --git a/doc/ci/components/index.md b/doc/ci/components/index.md index 338e4b2c205..0ebfb974afb 100644 --- a/doc/ci/components/index.md +++ b/doc/ci/components/index.md @@ -4,14 +4,12 @@ group: Pipeline Authoring info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# CI/CD components **(FREE ALL EXPERIMENT)** +# CI/CD components **(FREE ALL BETA)** > - Introduced as an [experimental feature](../../policy/experiment-beta-support.md) in GitLab 16.0, [with a flag](../../administration/feature_flags.md) named `ci_namespace_catalog_experimental`. Disabled by default. > - [Enabled on GitLab.com and self-managed](https://gitlab.com/groups/gitlab-org/-/epics/9897) in GitLab 16.2. > - [Feature flag `ci_namespace_catalog_experimental` removed](https://gitlab.com/gitlab-org/gitlab/-/issues/394772) in GitLab 16.3. - -This feature is an experimental feature and [an epic exists](https://gitlab.com/groups/gitlab-org/-/epics/9897) -to track future work. Tell us about your use case by leaving comments in the epic. +> - [Moved](https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_requests/130824) to [Beta status](../../policy/experiment-beta-support.md) in GitLab 16.6. A CI/CD component is a reusable single pipeline configuration unit. Use them to compose an entire pipeline configuration or a small part of a larger pipeline. diff --git a/doc/development/ai_architecture.md b/doc/development/ai_architecture.md index a41d17887ca..54ad52f0c39 100644 --- a/doc/development/ai_architecture.md +++ b/doc/development/ai_architecture.md @@ -55,9 +55,8 @@ It is possible to utilize other models or technologies, however they will need t The following models have been approved for use: -- [OpenAI models](https://platform.openai.com/docs/models) - Google's [Vertex AI](https://cloud.google.com/vertex-ai) and [model garden](https://cloud.google.com/model-garden) -- [AI Code Suggestions](https://gitlab.com/gitlab-org/modelops/applied-ml/code-suggestions/ai-assist/-/tree/main) +- [Anthropic models](https://docs.anthropic.com/claude/reference/selecting-a-model) - [Suggested reviewer](https://gitlab.com/gitlab-org/modelops/applied-ml/applied-ml-updates/-/issues/10) ### Vector stores diff --git a/doc/development/ai_features/duo_chat.md b/doc/development/ai_features/duo_chat.md index 2a624d4b830..ad044f4a923 100644 --- a/doc/development/ai_features/duo_chat.md +++ b/doc/development/ai_features/duo_chat.md @@ -12,7 +12,6 @@ NOTE: Use [this snippet](https://gitlab.com/gitlab-org/gitlab/-/snippets/2554994) for help automating the following section. 1. [Enable Anthropic API features](index.md#configure-anthropic-access). -1. [Enable OpenAI support](index.md#configure-openai-access). 1. [Ensure the embedding database is configured](index.md#set-up-the-embedding-database). 1. Ensure that your current branch is up-to-date with `master`. 1. To access the GitLab Duo Chat interface, in the lower-left corner of any page, select **Help** and **Ask GitLab Duo Chat**. @@ -97,10 +96,8 @@ functionality, you can use the following RSpec tests to validate answers to some predefined questions when using real LLMs: ```ruby -export OPENAI_EMBEDDINGS='true' # if using OpenAI embeddings export VERTEX_AI_EMBEDDINGS='true' # if using Vertex embeddings -export ANTHROPIC_API_KEY='<key>' # can use dev value of Gitlab::CurrentSettings.openai_api_key -export OPENAI_API_KEY='<key>' # can use dev value of Gitlab::CurrentSettings.anthropic_api_key +export ANTHROPIC_API_KEY='<key>' # can use dev value of Gitlab::CurrentSettings export VERTEX_AI_CREDENTIALS='<vertex-ai-credentials>' # can set as dev value of Gitlab::CurrentSettings.vertex_ai_credentials export VERTEX_AI_PROJECT='<vertex-project-name>' # can use dev value of Gitlab::CurrentSettings.vertex_ai_project @@ -113,7 +110,7 @@ make sure a new fixture is generated and committed together with the change. ## Running the rspecs tagged with `real_ai_request` The rspecs tagged with the metadata `real_ai_request` can be run in GitLab project's CI by triggering -`rspec-ee unit gitlab-duo-chat` or `rspec-ee unit gitlab-duo-chat-open-ai`. +`rspec-ee unit gitlab-duo-chat`. The former runs with Vertex APIs enabled. The CI jobs are optional and allowed to fail to account for the non-deterministic nature of LLM responses. diff --git a/doc/development/ai_features/index.md b/doc/development/ai_features/index.md index 8c90ae6cc85..df1627f2dc3 100644 --- a/doc/development/ai_features/index.md +++ b/doc/development/ai_features/index.md @@ -15,7 +15,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w - Background workers execute - GraphQL subscriptions deliver results back in real time - Abstraction for - - OpenAI - Google Vertex AI - Anthropic - Rate Limiting @@ -28,7 +27,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w - Automatic Markdown Rendering of responses - Centralised Group Level settings for experiment and 3rd party - Experimental API endpoints for exploration of AI APIs by GitLab team members without the need for credentials - - OpenAI - Google Vertex AI - Anthropic @@ -70,7 +68,7 @@ Use [this snippet](https://gitlab.com/gitlab-org/gitlab/-/snippets/2554994) for 1. Enable the specific feature flag for the feature you want to test 1. Set the required access token. To receive an access token: 1. For Vertex, follow the [instructions below](#configure-gcp-vertex-access). - 1. For all other providers, like Anthropic or OpenAI, create an access request where `@m_gill`, `@wayne`, and `@timzallmann` are the tech stack owners. + 1. For all other providers, like Anthropic, create an access request where `@m_gill`, `@wayne`, and `@timzallmann` are the tech stack owners. ### Set up the embedding database @@ -116,12 +114,6 @@ In order to obtain a GCP service key for local development, please follow the st Gitlab::CurrentSettings.update(vertex_ai_project: PROJECT_ID) ``` -### Configure OpenAI access - -```ruby -Gitlab::CurrentSettings.update(openai_api_key: "<open-ai-key>") -``` - ### Configure Anthropic access ```ruby @@ -182,9 +174,6 @@ Use the [experimental REST API endpoints](https://gitlab.com/gitlab-org/gitlab/- The endpoints are: -- `https://gitlab.example.com/api/v4/ai/experimentation/openai/completions` -- `https://gitlab.example.com/api/v4/ai/experimentation/openai/embeddings` -- `https://gitlab.example.com/api/v4/ai/experimentation/openai/chat/completions` - `https://gitlab.example.com/api/v4/ai/experimentation/anthropic/complete` - `https://gitlab.example.com/api/v4/ai/experimentation/vertex/chat` @@ -229,11 +218,9 @@ mutation { } ``` -The GraphQL API then uses the [OpenAI Client](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/gitlab/llm/open_ai/client.rb) +The GraphQL API then uses the [Anthropic Client](https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/lib/gitlab/llm/anthropic/client.rb) to send the response. -Remember that other clients are available and you should not use OpenAI. - #### How to receive a response The API requests to AI providers are handled in a background job. We therefore do not keep the request alive and the Frontend needs to match the request to the response from the subscription. @@ -274,7 +261,7 @@ To not have many concurrent subscriptions, you should also only subscribe to the #### Current abstraction layer flow -The following graph uses OpenAI as an example. You can use different providers. +The following graph uses VertexAI as an example. You can use different providers. ```mermaid flowchart TD @@ -283,9 +270,9 @@ B --> C[Llm::ExecuteMethodService] C --> D[One of services, for example: Llm::GenerateSummaryService] D -->|scheduled| E[AI worker:Llm::CompletionWorker] E -->F[::Gitlab::Llm::Completions::Factory] -F -->G[`::Gitlab::Llm::OpenAi::Completions::...` class using `::Gitlab::Llm::OpenAi::Templates::...` class] -G -->|calling| H[Gitlab::Llm::OpenAi::Client] -H --> |response| I[::Gitlab::Llm::OpenAi::ResponseService] +F -->G[`::Gitlab::Llm::VertexAi::Completions::...` class using `::Gitlab::Llm::Templates::...` class] +G -->|calling| H[Gitlab::Llm::VertexAi::Client] +H --> |response| I[::Gitlab::Llm::GraphqlSubscriptionResponseService] I --> J[GraphqlTriggers.ai_completion_response] J --> K[::GitlabSchema.subscriptions.trigger] ``` @@ -495,33 +482,27 @@ To move the feature from the experimental phase to the beta phase, move the name ### Implement calls to AI APIs and the prompts The `CompletionWorker` will call the `Completions::Factory` which will initialize the Service and execute the actual call to the API. -In our example, we will use OpenAI and implement two new classes: +In our example, we will use VertexAI and implement two new classes: ```ruby -# /ee/lib/gitlab/llm/open_ai/completions/amazing_new_ai_feature.rb +# /ee/lib/gitlab/llm/vertex_ai/completions/amazing_new_ai_feature.rb module Gitlab module Llm - module OpenAi + module VertexAi module Completions - class AmazingNewAiFeature - def initialize(ai_prompt_class) - @ai_prompt_class = ai_prompt_class - end + class AmazingNewAiFeature < Gitlab::Llm::Completions::Base + def execute + prompt = ai_prompt_class.new(options[:user_input]).to_prompt - def execute(user, issue, options) - options = ai_prompt_class.get_options(options[:messages]) + response = Gitlab::Llm::VertexAi::Client.new(user).text(content: prompt) - ai_response = Gitlab::Llm::OpenAi::Client.new(user).chat(content: nil, **options) + response_modifier = ::Gitlab::Llm::VertexAi::ResponseModifiers::Predictions.new(response) - ::Gitlab::Llm::OpenAi::ResponseService.new(user, issue, ai_response, options: {}).execute( - Gitlab::Llm::OpenAi::ResponseModifiers::Chat.new - ) + ::Gitlab::Llm::GraphqlSubscriptionResponseService.new( + user, nil, response_modifier, options: response_options + ).execute end - - private - - attr_reader :ai_prompt_class end end end @@ -530,28 +511,23 @@ end ``` ```ruby -# /ee/lib/gitlab/llm/open_ai/templates/amazing_new_ai_feature.rb +# /ee/lib/gitlab/llm/vertex_ai/templates/amazing_new_ai_feature.rb module Gitlab module Llm - module OpenAi + module VertexAi module Templates class AmazingNewAiFeature - TEMPERATURE = 0.3 - - def self.get_options(messages) - system_content = <<-TEMPLATE - You are an assistant that writes code for the following input: - """ - TEMPLATE - - { - messages: [ - { role: "system", content: system_content }, - { role: "user", content: messages }, - ], - temperature: TEMPERATURE - } + def initialize(user_input) + @user_input = user_input + end + + def to_prompt + <<-PROMPT + You are an assistant that writes code for the following context: + + context: #{user_input} + PROMPT end end end diff --git a/doc/development/documentation/styleguide/word_list.md b/doc/development/documentation/styleguide/word_list.md index 96005e0ccd7..1888d72f991 100644 --- a/doc/development/documentation/styleguide/word_list.md +++ b/doc/development/documentation/styleguide/word_list.md @@ -392,9 +392,14 @@ Use **confirmation dialog** to describe the dialog that asks you to confirm an a Do not use **confirmation box** or **confirmation dialog box**. See also [**dialog**](#dialog). -## Container Registry +## container registry -Use title case for the GitLab Container Registry. +When documenting the GitLab container registry features and functionality, use lower case. + +Use: + +- The GitLab container registry supports A, B, and C. +- You can push a Docker image to your project's container registry. ## currently diff --git a/doc/integration/mattermost/index.md b/doc/integration/mattermost/index.md index 73f3140db2b..c8a58f0692f 100644 --- a/doc/integration/mattermost/index.md +++ b/doc/integration/mattermost/index.md @@ -338,6 +338,7 @@ Below is a list of Mattermost version changes for GitLab 14.0 and later: | GitLab version | Mattermost version | Notes | | :------------- | :----------------- | ---------------------------------------------------------------------------------------- | +| 16.6 | 9.1 | | | 16.5 | 9.0 | | | 16.4 | 8.1 | | | 16.3 | 8.0 | | diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d1109f02a2a..d9371513ac1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -237,6 +237,11 @@ msgid_plural "%d completed issues" msgstr[0] "" msgstr[1] "" +msgid "%d compliance framework selected" +msgid_plural "%d compliance frameworks selected" +msgstr[0] "" +msgstr[1] "" + msgid "%d contribution" msgid_plural "%d contributions" msgstr[0] "" @@ -4791,6 +4796,9 @@ msgstr "" msgid "All environments" msgstr "" +msgid "All frameworks selected" +msgstr "" + msgid "All groups" msgstr "" @@ -18487,6 +18495,9 @@ msgstr "" msgid "Enforce two-factor authentication for all user sign-ins." msgstr "" +msgid "Engine" +msgstr "" + msgid "Enhance security by storing service account keys in secret managers - learn more about %{docLinkStart}secret management with GitLab%{docLinkEnd}" msgstr "" @@ -20717,16 +20728,16 @@ msgstr "" msgid "FindFile|Switch branch/tag" msgstr "" -msgid "FindingsDrawer|Category:" +msgid "FindingsDrawer|Code Quality" msgstr "" -msgid "FindingsDrawer|Engine:" +msgid "FindingsDrawer|Code Quality Finding" msgstr "" -msgid "FindingsDrawer|Other locations:" +msgid "FindingsDrawer|Detected in pipeline" msgstr "" -msgid "FindingsDrawer|Severity:" +msgid "FindingsDrawer|SAST Finding" msgstr "" msgid "Fingerprint (MD5)" @@ -41897,6 +41908,9 @@ msgstr "" msgid "SAML|Your organization's SSO has been connected to your GitLab account" msgstr "" +msgid "SAST" +msgstr "" + msgid "SBOMs last updated" msgstr "" @@ -42999,6 +43013,9 @@ msgstr "" msgid "SecurityOrchestration|Choose approver type" msgstr "" +msgid "SecurityOrchestration|Choose framework labels" +msgstr "" + msgid "SecurityOrchestration|Choose specific role" msgstr "" @@ -43008,6 +43025,9 @@ msgstr "" msgid "SecurityOrchestration|Create more robust vulnerability rules and apply them to all your projects." msgstr "" +msgid "SecurityOrchestration|Create new framework label" +msgstr "" + msgid "SecurityOrchestration|Create policy" msgstr "" @@ -43065,6 +43085,9 @@ msgstr "" msgid "SecurityOrchestration|Failed to load cluster agents." msgstr "" +msgid "SecurityOrchestration|Failed to load compliance frameworks" +msgstr "" + msgid "SecurityOrchestration|Failed to load group projects" msgstr "" @@ -43128,6 +43151,9 @@ msgstr "" msgid "SecurityOrchestration|No actions defined - policy will not run." msgstr "" +msgid "SecurityOrchestration|No compliance frameworks" +msgstr "" + msgid "SecurityOrchestration|No description" msgstr "" @@ -43268,6 +43294,9 @@ msgstr "" msgid "SecurityOrchestration|Select exception branches" msgstr "" +msgid "SecurityOrchestration|Select frameworks" +msgstr "" + msgid "SecurityOrchestration|Select groups" msgstr "" @@ -50447,6 +50476,9 @@ msgstr "" msgid "Too many users found. Quick actions are limited to at most %{max_count} users" msgstr "" +msgid "Tool" +msgstr "" + msgid "TopNav|Explore" msgstr "" diff --git a/spec/finders/ci/catalog/resources/versions_finder_spec.rb b/spec/finders/ci/catalog/resources/versions_finder_spec.rb new file mode 100644 index 00000000000..b2418aa45dd --- /dev/null +++ b/spec/finders/ci/catalog/resources/versions_finder_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Catalog::Resources::VersionsFinder, feature_category: :pipeline_composition do + include_context 'when there are catalog resources with versions' + + let(:sort) { nil } + let(:latest) { nil } + let(:params) { { sort: sort, latest: latest }.compact } + + subject(:execute) { described_class.new([resource1, resource2], current_user, params).execute } + + it 'avoids N+1 queries when authorizing multiple catalog resources', :request_store do + control_count = ActiveRecord::QueryRecorder.new { execute } + + # A new user is required to avoid a false positive from cached user authorization queries + new_user = create(:user) + + expect do + described_class.new([resource1, resource2, resource3], new_user, params).execute + end.not_to exceed_query_limit(control_count) + end + + context 'when the user is not authorized for any catalog resource' do + it 'returns empty response' do + is_expected.to be_empty + end + end + + describe 'versions' do + before_all do + resource1.project.add_guest(current_user) + end + + it 'returns the versions of the authorized catalog resource' do + expect(execute).to match_array([v1_0, v1_1]) + end + + context 'with sort parameter' do + it 'returns versions ordered by released_at descending by default' do + expect(execute).to eq([v1_1, v1_0]) + end + + context 'when sort is released_at_asc' do + let(:sort) { 'released_at_asc' } + + it 'returns versions ordered by released_at ascending' do + expect(execute).to eq([v1_0, v1_1]) + end + end + + context 'when sort is created_asc' do + let(:sort) { 'created_asc' } + + it 'returns versions ordered by created_at ascending' do + expect(execute).to eq([v1_1, v1_0]) + end + end + + context 'when sort is created_desc' do + let(:sort) { 'created_desc' } + + it 'returns versions ordered by created_at descending' do + expect(execute).to eq([v1_0, v1_1]) + end + end + end + + it 'preloads associations' do + expect(Ci::Catalog::Resources::Version).to receive(:preloaded).once.and_call_original + + execute + end + end + + describe 'latest versions' do + before_all do + resource1.project.add_guest(current_user) + resource2.project.add_guest(current_user) + end + + let(:latest) { true } + + it 'returns the latest version for each authorized catalog resource' do + expect(execute).to match_array([v1_1, v2_1]) + end + + context 'when one catalog resource does not have versions' do + it 'returns the latest version of only the catalog resource with versions' do + resource1.versions.delete_all(:delete_all) + + is_expected.to match_array([v2_1]) + end + end + + context 'when no catalog resource has versions' do + it 'returns empty response' do + resource1.versions.delete_all(:delete_all) + resource2.versions.delete_all(:delete_all) + + is_expected.to be_empty + end + end + end +end diff --git a/spec/frontend/diffs/components/shared/__snapshots__/findings_drawer_spec.js.snap b/spec/frontend/diffs/components/shared/__snapshots__/findings_drawer_spec.js.snap index afa2a7d9678..cfc34bd2f25 100644 --- a/spec/frontend/diffs/components/shared/__snapshots__/findings_drawer_spec.js.snap +++ b/spec/frontend/diffs/components/shared/__snapshots__/findings_drawer_spec.js.snap @@ -1,111 +1,220 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`FindingsDrawer matches the snapshot 1`] = ` -<gl-drawer-stub +<transition-stub class="findings-drawer" - headerheight="" - open="true" - variant="default" - zindex="252" + name="gl-drawer" > - <h2 - class="gl-font-size-h2 gl-mb-0 gl-mt-0" - data-testid="findings-drawer-heading" + <aside + class="gl-drawer gl-drawer-default" + style="top: 0px; z-index: 252;" > - Unused method argument - \`c\`. If it's necessary, use \`_\` or \`_c\` as an argument name to indicate that it won't be used. - </h2> - <ul - class="gl-border-b-initial gl-list-style-none gl-mb-0 gl-pb-0!" - > - <li - class="gl-mb-4" - data-testid="findings-drawer-severity" - > - <span - class="gl-font-weight-bold" - > - Severity: - </span> - <gl-icon-stub - class="gl-text-orange-300 inline-findings-severity-icon" - data-testid="findings-drawer-severity-icon" - name="severity-low" - size="12" - /> - minor - </li> - <li - class="gl-mb-4" - data-testid="findings-drawer-engine" - > - <span - class="gl-font-weight-bold" - > - Engine: - </span> - testengine name - </li> - <li - class="gl-mb-4" - data-testid="findings-drawer-category" + <div + class="gl-drawer-header" > - <span - class="gl-font-weight-bold" + <div + class="gl-drawer-title" > - Category: - </span> - testcategory 1 - </li> - <li - class="gl-mb-4" - data-testid="findings-drawer-other-locations" + <h2 + class="drawer-heading gl-font-base gl-mb-0 gl-mt-0" + > + <svg + aria-hidden="true" + class="gl-icon gl-text-orange-300 gl-vertical-align-baseline! inline-findings-severity-icon s12" + data-testid="severity-low-icon" + role="img" + > + <use + href="file-mock#severity-low" + /> + </svg> + <span + class="drawer-heading-severity" + > + low + </span> + SAST Finding + </h2> + <button + aria-label="Close drawer" + class="btn btn-default btn-default-tertiary btn-icon btn-sm gl-button gl-drawer-close-button" + type="button" + > + <svg + aria-hidden="true" + class="gl-button-icon gl-icon s16" + data-testid="close-icon" + role="img" + > + <use + href="file-mock#close" + /> + </svg> + </button> + </div> + </div> + <div + class="gl-drawer-body gl-drawer-body-scrim" > - <span - class="gl-display-block gl-font-weight-bold gl-mb-3" - > - Other locations: - </span> <ul - class="gl-pl-6" + class="gl-border-b-initial gl-list-style-none gl-mb-0 gl-pb-0!" > <li - class="gl-mb-1" + class="gl-mb-4" + > + <p + class="gl-line-height-20" + > + <span + class="gl-display-block gl-font-weight-bold gl-mb-1" + data-testid="findings-drawer-item-description" + > + Name + </span> + <span + data-testid="findings-drawer-item-value-prop" + > + mockedtitle + </span> + </p> + </li> + <li + class="gl-mb-4" + > + <p + class="gl-line-height-20" + > + <span + class="gl-display-block gl-font-weight-bold gl-mb-1" + data-testid="findings-drawer-item-description" + > + Status + </span> + <span + class="badge badge-pill badge-warning gl-badge md text-capitalize" + > + detected + </span> + </p> + </li> + <li + class="gl-mb-4" + > + <p + class="gl-line-height-20" + > + <span + class="gl-display-block gl-font-weight-bold gl-mb-1" + data-testid="findings-drawer-item-description" + > + Description + </span> + <span + data-testid="findings-drawer-item-value-prop" + > + fakedesc + </span> + </p> + </li> + <li + class="gl-mb-4" + > + <p + class="gl-line-height-20" + > + <span + class="gl-display-block gl-font-weight-bold gl-mb-1" + data-testid="findings-drawer-item-description" + > + Project + </span> + <a + class="gl-link" + href="/testpath" + > + testname + </a> + </p> + </li> + <li + class="gl-mb-4" + > + <p + class="gl-line-height-20" + > + <span + class="gl-display-block gl-font-weight-bold gl-mb-1" + data-testid="findings-drawer-item-description" + > + File + </span> + <span + data-testid="findings-drawer-item-value-prop" + /> + </p> + </li> + <li + class="gl-mb-4" > - <gl-link-stub - data-testid="findings-drawer-other-locations-link" - href="http://testlink.com" + <p + class="gl-line-height-20" > - testpath - </gl-link-stub> + <span + class="gl-display-block gl-font-weight-bold gl-mb-1" + data-testid="findings-drawer-item-description" + > + Identifiers + </span> + <span> + <a + class="gl-link" + href="https://semgrep.dev/r/gitlab.eslint.detect-disable-mustache-escape" + > + eslint.detect-disable-mustache-escape + </a> + </span> + </p> </li> <li - class="gl-mb-1" + class="gl-mb-4" > - <gl-link-stub - data-testid="findings-drawer-other-locations-link" - href="http://testlink.com" + <p + class="gl-line-height-20" > - testpath 1 - </gl-link-stub> + <span + class="gl-display-block gl-font-weight-bold gl-mb-1" + data-testid="findings-drawer-item-description" + > + Tool + </span> + <span + data-testid="findings-drawer-item-value-prop" + > + SAST + </span> + </p> </li> <li - class="gl-mb-1" + class="gl-mb-4" > - <gl-link-stub - data-testid="findings-drawer-other-locations-link" - href="http://testlink.com" + <p + class="gl-line-height-20" > - testpath2 - </gl-link-stub> + <span + class="gl-display-block gl-font-weight-bold gl-mb-1" + data-testid="findings-drawer-item-description" + > + Engine + </span> + <span + data-testid="findings-drawer-item-value-prop" + > + testengine name + </span> + </p> </li> </ul> - </li> - </ul> - <span - class="drawer-body gl-display-block gl-px-3 gl-py-0!" - data-testid="findings-drawer-body" - > - Duplicated Code Duplicated code - </span> -</gl-drawer-stub> + </div> + </aside> +</transition-stub> `; diff --git a/spec/frontend/diffs/components/shared/findings_drawer_item_spec.js b/spec/frontend/diffs/components/shared/findings_drawer_item_spec.js new file mode 100644 index 00000000000..80087ea66a2 --- /dev/null +++ b/spec/frontend/diffs/components/shared/findings_drawer_item_spec.js @@ -0,0 +1,54 @@ +import FindingsDrawerItem from '~/diffs/components/shared/findings_drawer_item.vue'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; + +let wrapper; + +const mockDescription = 'testDescription'; +const slotTestId = 'findings-drawer-item-value-slot'; +const mockValue = 'testValue'; +const mockSlot = `<span data-testid="${slotTestId}">mockSlot</span>`; +const mockSlotText = 'mockSlot'; + +describe('FindingsDrawerItem', () => { + const description = () => wrapper.findByTestId('findings-drawer-item-description'); + + const valueSlot = () => wrapper.findByTestId(slotTestId); + const valueProp = () => wrapper.findByTestId('findings-drawer-item-value-prop'); + + const createWrapper = (props = {}, slots = {}) => { + return shallowMountExtended(FindingsDrawerItem, { + propsData: { + ...props, + }, + slots: { + ...slots, + }, + }); + }; + + it('renders with default values', () => { + wrapper = createWrapper(); + expect(description().text()).toContain(''); + expect(valueProp().text()).toContain(''); + }); + + it('renders description and value props correctly', () => { + wrapper = createWrapper({ description: mockDescription, value: mockValue }); + expect(description().text()).toContain(mockDescription); + expect(valueProp().text()).toContain(mockValue); + }); + + describe('when slot content is passed', () => { + it('renders slot content', () => { + wrapper = createWrapper({}, { value: mockSlot }); + expect(valueSlot().text()).toContain(mockSlotText); + }); + + describe('when value prop is passed', () => { + it('does not render value prop', () => { + wrapper = createWrapper({ value: mockValue }, { value: mockSlot }); + expect(valueProp().exists()).toBe(false); + }); + }); + }); +}); diff --git a/spec/frontend/diffs/components/shared/findings_drawer_spec.js b/spec/frontend/diffs/components/shared/findings_drawer_spec.js index 0af6e0f0e96..62d875ed9b7 100644 --- a/spec/frontend/diffs/components/shared/findings_drawer_spec.js +++ b/spec/frontend/diffs/components/shared/findings_drawer_spec.js @@ -1,16 +1,33 @@ +import { GlDrawer } from '@gitlab/ui'; import FindingsDrawer from '~/diffs/components/shared/findings_drawer.vue'; -import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; -import mockFinding from '../../mock_data/findings_drawer'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import { mockFinding, mockProject } from '../../mock_data/findings_drawer'; let wrapper; +const getDrawer = () => wrapper.findComponent(GlDrawer); +const closeEvent = 'close'; + +const createWrapper = () => { + return mountExtended(FindingsDrawer, { + propsData: { + drawer: mockFinding, + project: mockProject, + }, + }); +}; + describe('FindingsDrawer', () => { - const createWrapper = () => { - return shallowMountExtended(FindingsDrawer, { - propsData: { - drawer: mockFinding, - }, - }); - }; + it('renders without errors', () => { + wrapper = createWrapper(); + expect(wrapper.exists()).toBe(true); + }); + + it('emits close event when gl-drawer emits close event', () => { + wrapper = createWrapper(); + + getDrawer().vm.$emit(closeEvent); + expect(wrapper.emitted(closeEvent)).toHaveLength(1); + }); it('matches the snapshot', () => { wrapper = createWrapper(); diff --git a/spec/frontend/diffs/mock_data/findings_drawer.js b/spec/frontend/diffs/mock_data/findings_drawer.js index d7e7e957c83..4823a18b267 100644 --- a/spec/frontend/diffs/mock_data/findings_drawer.js +++ b/spec/frontend/diffs/mock_data/findings_drawer.js @@ -1,21 +1,28 @@ -export default { +export const mockFinding = { + title: 'mockedtitle', + state: 'detected', + scale: 'sast', line: 7, - description: - "Unused method argument - `c`. If it's necessary, use `_` or `_c` as an argument name to indicate that it won't be used.", - severity: 'minor', + description: 'fakedesc', + severity: 'low', engineName: 'testengine name', categories: ['testcategory 1', 'testcategory 2'], content: { body: 'Duplicated Code Duplicated code', }, - location: { - path: 'workhorse/config_test.go', - lines: { begin: 221, end: 284 }, - }, - otherLocations: [ - { path: 'testpath', href: 'http://testlink.com' }, - { path: 'testpath 1', href: 'http://testlink.com' }, - { path: 'testpath2', href: 'http://testlink.com' }, + webUrl: {}, + identifiers: [ + { + __typename: 'VulnerabilityIdentifier', + externalId: 'eslint.detect-disable-mustache-escape', + externalType: 'semgrep_id', + name: 'eslint.detect-disable-mustache-escape', + url: 'https://semgrep.dev/r/gitlab.eslint.detect-disable-mustache-escape', + }, ], - type: 'issue', +}; + +export const mockProject = { + nameWithNamespace: 'testname', + fullPath: 'testpath', }; diff --git a/spec/frontend/diffs/utils/sort_errors_by_file_spec.js b/spec/frontend/diffs/utils/sort_findings_by_file_spec.js index ca8a8ec3516..8dc4f57d98c 100644 --- a/spec/frontend/diffs/utils/sort_errors_by_file_spec.js +++ b/spec/frontend/diffs/utils/sort_findings_by_file_spec.js @@ -6,6 +6,10 @@ describe('sort_findings_by_file utilities', () => { const mockLine = '00'; const mockFile1 = 'file1.js'; const mockFile2 = 'file2.rb'; + const webUrl1 = 'http://example.com/file1.js'; + const webUrl2 = 'http://example.com/file2.rb'; + const engineName1 = 'engineName1'; + const engineName2 = 'engineName2'; const emptyResponse = { files: {}, }; @@ -16,12 +20,16 @@ describe('sort_findings_by_file utilities', () => { filePath: mockFile1, line: mockLine, description: mockDescription, + webUrl: webUrl1, + engineName: engineName1, }, { severity: mockSeverity, filePath: mockFile2, line: mockLine, description: mockDescription, + webUrl: webUrl2, + engineName: engineName2, }, ]; const sortedFindings = { @@ -29,15 +37,21 @@ describe('sort_findings_by_file utilities', () => { [mockFile1]: [ { line: mockLine, + filePath: mockFile1, description: mockDescription, severity: mockSeverity, + webUrl: webUrl1, + engineName: engineName1, }, ], [mockFile2]: [ { line: mockLine, + filePath: mockFile2, description: mockDescription, severity: mockSeverity, + webUrl: webUrl2, + engineName: engineName2, }, ], }, diff --git a/spec/models/ci/catalog/resource_spec.rb b/spec/models/ci/catalog/resource_spec.rb index 14c9d95bb11..098772b1ea9 100644 --- a/spec/models/ci/catalog/resource_spec.rb +++ b/spec/models/ci/catalog/resource_spec.rb @@ -110,18 +110,6 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do end end - describe '#versions' do - it 'returns releases ordered by released date descending' do - expect(resource.versions).to eq([release3, release2, release1]) - end - end - - describe '#latest_version' do - it 'returns the latest release' do - expect(resource.latest_version).to eq(release3) - end - end - describe '#state' do it 'defaults to draft' do expect(resource.state).to eq('draft') diff --git a/spec/models/ci/catalog/resources/version_spec.rb b/spec/models/ci/catalog/resources/version_spec.rb index e93176e466a..7114d2b6709 100644 --- a/spec/models/ci/catalog/resources/version_spec.rb +++ b/spec/models/ci/catalog/resources/version_spec.rb @@ -3,14 +3,105 @@ require 'spec_helper' RSpec.describe Ci::Catalog::Resources::Version, type: :model, feature_category: :pipeline_composition do + include_context 'when there are catalog resources with versions' + it { is_expected.to belong_to(:release) } it { is_expected.to belong_to(:catalog_resource).class_name('Ci::Catalog::Resource') } it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:components).class_name('Ci::Catalog::Resources::Component') } + it { is_expected.to delegate_method(:name).to(:release) } + it { is_expected.to delegate_method(:description).to(:release) } + it { is_expected.to delegate_method(:tag).to(:release) } + it { is_expected.to delegate_method(:sha).to(:release) } + it { is_expected.to delegate_method(:released_at).to(:release) } + it { is_expected.to delegate_method(:author_id).to(:release) } + describe 'validations' do it { is_expected.to validate_presence_of(:release) } it { is_expected.to validate_presence_of(:catalog_resource) } it { is_expected.to validate_presence_of(:project) } end + + describe '.for_catalog resources' do + it 'returns versions for the given catalog resources' do + versions = described_class.for_catalog_resources([resource1, resource2]) + + expect(versions).to match_array([v1_0, v1_1, v2_0, v2_1]) + end + end + + describe '.order_by_created_at_asc' do + it 'returns versions ordered by created_at ascending' do + versions = described_class.order_by_created_at_asc + + expect(versions).to eq([v2_1, v2_0, v1_1, v1_0]) + end + end + + describe '.order_by_created_at_desc' do + it 'returns versions ordered by created_at descending' do + versions = described_class.order_by_created_at_desc + + expect(versions).to eq([v1_0, v1_1, v2_0, v2_1]) + end + end + + describe '.order_by_released_at_asc' do + it 'returns versions ordered by released_at ascending' do + versions = described_class.order_by_released_at_asc + + expect(versions).to eq([v1_0, v1_1, v2_0, v2_1]) + end + end + + describe '.order_by_released_at_desc' do + it 'returns versions ordered by released_at descending' do + versions = described_class.order_by_released_at_desc + + expect(versions).to eq([v2_1, v2_0, v1_1, v1_0]) + end + end + + describe '.latest' do + subject { described_class.latest } + + it 'returns the latest version by released date' do + is_expected.to eq(v2_1) + end + + context 'when there are no versions' do + it 'returns nil' do + resource1.versions.delete_all(:delete_all) + resource2.versions.delete_all(:delete_all) + + is_expected.to be_nil + end + end + end + + describe '.latest_for_catalog resources' do + subject { described_class.latest_for_catalog_resources([resource1, resource2]) } + + it 'returns the latest version for each catalog resource' do + is_expected.to match_array([v1_1, v2_1]) + end + + context 'when one catalog resource does not have versions' do + it 'returns the latest version of only the catalog resource with versions' do + resource1.versions.delete_all(:delete_all) + + is_expected.to match_array([v2_1]) + end + end + + context 'when no catalog resource has versions' do + it 'returns empty response' do + resource1.versions.delete_all(:delete_all) + resource2.versions.delete_all(:delete_all) + + is_expected.to be_empty + end + end + end end diff --git a/spec/models/concerns/enums/sbom_spec.rb b/spec/models/concerns/enums/sbom_spec.rb index 4cfd50b6c8a..e2f56cc637d 100644 --- a/spec/models/concerns/enums/sbom_spec.rb +++ b/spec/models/concerns/enums/sbom_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Enums::Sbom, feature_category: :dependency_management do :apk | 9 :rpm | 10 :deb | 11 - :cbl_mariner | 12 + 'cbl-mariner' | 12 :wolfi | 13 'unknown-pkg-manager' | 0 'Python (unknown)' | 0 diff --git a/spec/models/projects/repository_storage_move_spec.rb b/spec/models/projects/repository_storage_move_spec.rb index ab0ad81f77a..c5fbc92176f 100644 --- a/spec/models/projects/repository_storage_move_spec.rb +++ b/spec/models/projects/repository_storage_move_spec.rb @@ -3,33 +3,11 @@ require 'spec_helper' RSpec.describe Projects::RepositoryStorageMove, type: :model do - let_it_be_with_refind(:project) { create(:project) } - it_behaves_like 'handles repository moves' do - let(:container) { project } + let_it_be_with_refind(:container) { create(:project) } + let(:repository_storage_factory_key) { :project_repository_storage_move } let(:error_key) { :project } let(:repository_storage_worker) { Projects::UpdateRepositoryStorageWorker } end - - describe 'state transitions' do - let(:storage) { 'test_second_storage' } - - before do - stub_storage_settings(storage => { 'path' => 'tmp/tests/extra_storage' }) - end - - context 'when started' do - subject(:storage_move) { create(:project_repository_storage_move, :started, container: project, destination_storage_name: storage) } - - context 'and transits to replicated' do - it 'sets the repository storage and marks the container as writable' do - storage_move.finish_replication! - - expect(project.repository_storage).to eq(storage) - expect(project).not_to be_repository_read_only - end - end - end - end end diff --git a/spec/support/shared_contexts/ci/catalog/resources/version_shared_context.rb b/spec/support/shared_contexts/ci/catalog/resources/version_shared_context.rb new file mode 100644 index 00000000000..3c9bb980b46 --- /dev/null +++ b/spec/support/shared_contexts/ci/catalog/resources/version_shared_context.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +RSpec.shared_context 'when there are catalog resources with versions' do + let_it_be(:current_user) { create(:user) } + + let_it_be(:project1) { create(:project, :repository) } + let_it_be(:project2) { create(:project, :repository) } + let_it_be(:project3) { create(:project, :repository) } + let_it_be_with_reload(:resource1) { create(:ci_catalog_resource, project: project1) } + let_it_be_with_reload(:resource2) { create(:ci_catalog_resource, project: project2) } + let_it_be(:resource3) { create(:ci_catalog_resource, project: project3) } + + let_it_be(:release_v1_0) { create(:release, project: project1, tag: 'v1.0', released_at: 4.days.ago) } + let_it_be(:release_v1_1) { create(:release, project: project1, tag: 'v1.1', released_at: 3.days.ago) } + let_it_be(:release_v2_0) { create(:release, project: project2, tag: 'v2.0', released_at: 2.days.ago) } + let_it_be(:release_v2_1) { create(:release, project: project2, tag: 'v2.1', released_at: 1.day.ago) } + + let_it_be(:v1_0) do + create(:ci_catalog_resource_version, catalog_resource: resource1, release: release_v1_0, created_at: 1.day.ago) + end + + let_it_be(:v1_1) do + create(:ci_catalog_resource_version, catalog_resource: resource1, release: release_v1_1, created_at: 2.days.ago) + end + + let_it_be(:v2_0) do + create(:ci_catalog_resource_version, catalog_resource: resource2, release: release_v2_0, created_at: 3.days.ago) + end + + let_it_be(:v2_1) do + create(:ci_catalog_resource_version, catalog_resource: resource2, release: release_v2_1, created_at: 4.days.ago) + end +end diff --git a/spec/support/shared_examples/models/concerns/repository_storage_movable_shared_examples.rb b/spec/support/shared_examples/models/concerns/repository_storage_movable_shared_examples.rb index bb382ec511c..d8a8d1e1cea 100644 --- a/spec/support/shared_examples/models/concerns/repository_storage_movable_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/repository_storage_movable_shared_examples.rb @@ -72,9 +72,9 @@ RSpec.shared_examples 'handles repository moves' do end context 'when in the default state' do - subject(:storage_move) { create(repository_storage_factory_key, container: container, destination_storage_name: 'test_second_storage') } + let!(:storage_move) { create(repository_storage_factory_key, container: container, destination_storage_name: 'test_second_storage') } - context 'and transits to scheduled' do + context 'and transitions to scheduled' do it 'triggers the corresponding repository storage worker' do expect(repository_storage_worker).to receive(:perform_async).with(container.id, 'test_second_storage', storage_move.id) @@ -90,31 +90,37 @@ RSpec.shared_examples 'handles repository moves' do it 'does not trigger the corresponding repository storage worker and adds an error' do expect(repository_storage_worker).not_to receive(:perform_async) + storage_move.schedule! + expect(storage_move.errors[error_key]).to include('foobar') end it 'sets the state to failed' do expect(storage_move).to receive(:do_fail!).and_call_original + storage_move.schedule! + expect(storage_move.state_name).to eq(:failed) + expect(container).not_to be_repository_read_only end end end - context 'and transits to started' do + context 'and transitions to started' do it 'does not allow the transition' do - expect { storage_move.start! } - .to raise_error(StateMachines::InvalidTransition) + expect { storage_move.start! }.to raise_error(StateMachines::InvalidTransition) end end end context 'when started' do - subject(:storage_move) { create(repository_storage_factory_key, :started, container: container, destination_storage_name: 'test_second_storage') } + let!(:storage_move) { create(repository_storage_factory_key, :started, container: container, destination_storage_name: 'test_second_storage') } - context 'and transits to replicated' do + context 'and transitions to replicated' do it 'marks the container as writable' do + container.set_repository_read_only! + storage_move.finish_replication! expect(container).not_to be_repository_read_only @@ -122,12 +128,29 @@ RSpec.shared_examples 'handles repository moves' do it 'updates the updated_at column of the container', :aggregate_failures do expect { storage_move.finish_replication! }.to change { container.updated_at } + expect(storage_move.container.updated_at).to be >= storage_move.updated_at end end - context 'and transits to failed' do + context 'and transitions to failed' do it 'marks the container as writable' do + container.set_repository_read_only! + + storage_move.do_fail! + + expect(container).not_to be_repository_read_only + end + end + end + + context 'when replicated' do + let!(:storage_move) { create(repository_storage_factory_key, :replicated, container: container, destination_storage_name: 'test_second_storage') } + + context 'and transitions to cleanup_failed' do + it 'marks the container as writable' do + container.set_repository_read_only! + storage_move.do_fail! expect(container).not_to be_repository_read_only |