diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-11-08 15:12:07 +0300 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-11-08 15:12:07 +0300 |
| commit | 6a380347147d1a55afbc6a1c16e04b567ab90d86 (patch) | |
| tree | 9854eb7894cbe8a47976cce0289aa861df79eee3 | |
| parent | 4901ff1764398bb017487d4a5104b74bc284f33a (diff) | |
Add latest changes from gitlab-org/gitlab@master
64 files changed, 1675 insertions, 610 deletions
diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml index d19300f8e8d..74fc5f2cdc0 100644 --- a/.gitlab/ci/qa.gitlab-ci.yml +++ b/.gitlab/ci/qa.gitlab-ci.yml @@ -58,10 +58,13 @@ update-qa-cache: - tooling/bin/find_change_diffs ${CHANGES_DIFFS_DIR} script: - | - if tooling/bin/qa/check_if_only_quarantined_specs ${CHANGES_DIFFS_DIR}; then - exit 0 - else + tooling/bin/qa/package_and_qa_check ${CHANGES_DIFFS_DIR} && exit_code=$? + if [ $exit_code -eq 0 ]; then ./scripts/trigger-build omnibus + elif [ $exit_code -eq 1 ]; then + exit 1 + else + echo "Downstream jobs will not be triggered because package_and_qa_check exited with code: $exit_code" fi # These jobs often time out, so temporarily use private runners and a long timeout: https://gitlab.com/gitlab-org/gitlab/-/issues/238563 tags: diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b8363ed543..8098e98d4aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.4.2 (2021-11-08) + +### Fixed (3 changes) + +- [Skip retrying for reads on connection errors if primary only](gitlab-org/gitlab@8e1976ed75bd6c606d49c83863cf46bf3c4d5070) ([merge request](gitlab-org/gitlab!73919)) +- [Fix error 500 loading branch with UTF-8 characters with performance bar](gitlab-org/gitlab@67ddc428472d57bb3d8a4a84eb0750487a175f75) ([merge request](gitlab-org/gitlab!73919)) +- [Skip st_diff callback setting on LegacyDiffNote when importing](gitlab-org/gitlab@84f5c66321473cd702b3b671584054fcf3d141ae) ([merge request](gitlab-org/gitlab!73919)) + +### Changed (1 change) + +- [Remove skip_legacy_diff_note_callback_on_import from legacy diff note](gitlab-org/gitlab@547a2ec29ea9e9299eab727899c3d90886ffc21c) ([merge request](gitlab-org/gitlab!73919)) + +### Performance (1 change) + +- [Prevent Sidekiq size limiter middleware from running multiple times on the same job](gitlab-org/gitlab@294c01be38d400607536fb20a2038e098c0f0e28) ([merge request](gitlab-org/gitlab!73919)) + ## 14.4.1 (2021-10-28) ### Security (13 changes) diff --git a/app/assets/javascripts/runner/components/cells/runner_actions_cell.vue b/app/assets/javascripts/runner/components/cells/runner_actions_cell.vue index e26bdbf1aea..c4bddb7b398 100644 --- a/app/assets/javascripts/runner/components/cells/runner_actions_cell.vue +++ b/app/assets/javascripts/runner/components/cells/runner_actions_cell.vue @@ -3,7 +3,7 @@ import { GlButton, GlButtonGroup, GlTooltipDirective } from '@gitlab/ui'; import createFlash from '~/flash'; import { __, s__ } from '~/locale'; import runnerDeleteMutation from '~/runner/graphql/runner_delete.mutation.graphql'; -import runnerUpdateMutation from '~/runner/graphql/runner_update.mutation.graphql'; +import runnerActionsUpdateMutation from '~/runner/graphql/runner_actions_update.mutation.graphql'; import { captureException } from '~/runner/sentry_utils'; const i18n = { @@ -71,7 +71,7 @@ export default { runnerUpdate: { errors }, }, } = await this.$apollo.mutate({ - mutation: runnerUpdateMutation, + mutation: runnerActionsUpdateMutation, variables: { input: { id: this.runner.id, diff --git a/app/assets/javascripts/runner/components/cells/runner_status_cell.vue b/app/assets/javascripts/runner/components/cells/runner_status_cell.vue new file mode 100644 index 00000000000..9ba1192bc8c --- /dev/null +++ b/app/assets/javascripts/runner/components/cells/runner_status_cell.vue @@ -0,0 +1,40 @@ +<script> +import { GlTooltipDirective } from '@gitlab/ui'; + +import RunnerContactedStateBadge from '../runner_contacted_state_badge.vue'; +import RunnerPausedBadge from '../runner_paused_badge.vue'; + +import { I18N_LOCKED_RUNNER_DESCRIPTION, I18N_PAUSED_RUNNER_DESCRIPTION } from '../../constants'; + +export default { + components: { + RunnerContactedStateBadge, + RunnerPausedBadge, + }, + directives: { + GlTooltip: GlTooltipDirective, + }, + props: { + runner: { + type: Object, + required: true, + }, + }, + computed: { + paused() { + return !this.runner.active; + }, + }, + i18n: { + I18N_LOCKED_RUNNER_DESCRIPTION, + I18N_PAUSED_RUNNER_DESCRIPTION, + }, +}; +</script> + +<template> + <div> + <runner-contacted-state-badge :runner="runner" size="sm" /> + <runner-paused-badge v-if="paused" size="sm" /> + </div> +</template> diff --git a/app/assets/javascripts/runner/components/cells/runner_summary_cell.vue b/app/assets/javascripts/runner/components/cells/runner_summary_cell.vue index 886b5cb29fc..3b476997915 100644 --- a/app/assets/javascripts/runner/components/cells/runner_summary_cell.vue +++ b/app/assets/javascripts/runner/components/cells/runner_summary_cell.vue @@ -1,11 +1,21 @@ <script> +import { GlIcon, GlTooltipDirective } from '@gitlab/ui'; + import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue'; import RunnerName from '../runner_name.vue'; +import RunnerTypeBadge from '../runner_type_badge.vue'; + +import { I18N_LOCKED_RUNNER_DESCRIPTION } from '../../constants'; export default { components: { + GlIcon, TooltipOnTruncate, RunnerName, + RunnerTypeBadge, + }, + directives: { + GlTooltip: GlTooltipDirective, }, props: { runner: { @@ -14,10 +24,19 @@ export default { }, }, computed: { + runnerType() { + return this.runner.runnerType; + }, + locked() { + return this.runner.locked; + }, description() { return this.runner.description; }, }, + i18n: { + I18N_LOCKED_RUNNER_DESCRIPTION, + }, }; </script> @@ -26,6 +45,14 @@ export default { <slot :runner="runner" name="runner-name"> <runner-name :runner="runner" /> </slot> + + <runner-type-badge :type="runnerType" size="sm" /> + <gl-icon + v-if="locked" + v-gl-tooltip + :title="$options.i18n.I18N_LOCKED_RUNNER_DESCRIPTION" + name="lock" + /> <tooltip-on-truncate class="gl-display-block" :title="description" truncate-target="child"> <div class="gl-text-truncate"> {{ description }} diff --git a/app/assets/javascripts/runner/components/cells/runner_type_cell.vue b/app/assets/javascripts/runner/components/cells/runner_type_cell.vue deleted file mode 100644 index c8cb0bf6088..00000000000 --- a/app/assets/javascripts/runner/components/cells/runner_type_cell.vue +++ /dev/null @@ -1,47 +0,0 @@ -<script> -import { GlTooltipDirective } from '@gitlab/ui'; -import RunnerTypeBadge from '../runner_type_badge.vue'; -import RunnerStateLockedBadge from '../runner_state_locked_badge.vue'; -import RunnerStatePausedBadge from '../runner_state_paused_badge.vue'; -import { I18N_LOCKED_RUNNER_DESCRIPTION, I18N_PAUSED_RUNNER_DESCRIPTION } from '../../constants'; - -export default { - components: { - RunnerTypeBadge, - RunnerStateLockedBadge, - RunnerStatePausedBadge, - }, - directives: { - GlTooltip: GlTooltipDirective, - }, - props: { - runner: { - type: Object, - required: true, - }, - }, - computed: { - runnerType() { - return this.runner.runnerType; - }, - locked() { - return this.runner.locked; - }, - paused() { - return !this.runner.active; - }, - }, - i18n: { - I18N_LOCKED_RUNNER_DESCRIPTION, - I18N_PAUSED_RUNNER_DESCRIPTION, - }, -}; -</script> - -<template> - <div> - <runner-type-badge :type="runnerType" size="sm" /> - <runner-state-locked-badge v-if="locked" size="sm" /> - <runner-state-paused-badge v-if="paused" size="sm" /> - </div> -</template> diff --git a/app/assets/javascripts/runner/components/runner_contacted_state_badge.vue b/app/assets/javascripts/runner/components/runner_contacted_state_badge.vue new file mode 100644 index 00000000000..b4727f832f8 --- /dev/null +++ b/app/assets/javascripts/runner/components/runner_contacted_state_badge.vue @@ -0,0 +1,69 @@ +<script> +import { GlBadge, GlTooltipDirective } from '@gitlab/ui'; +import { s__, sprintf } from '~/locale'; +import { getTimeago } from '~/lib/utils/datetime_utility'; +import { + I18N_ONLINE_RUNNER_DESCRIPTION, + I18N_OFFLINE_RUNNER_DESCRIPTION, + I18N_NOT_CONNECTED_RUNNER_DESCRIPTION, + STATUS_ONLINE, + STATUS_OFFLINE, + STATUS_NOT_CONNECTED, +} from '../constants'; + +export default { + components: { + GlBadge, + }, + directives: { + GlTooltip: GlTooltipDirective, + }, + props: { + runner: { + required: true, + type: Object, + }, + }, + computed: { + contactedAtTimeAgo() { + if (this.runner.contactedAt) { + return getTimeago().format(this.runner.contactedAt); + } + return null; + }, + badge() { + switch (this.runner.status) { + case STATUS_ONLINE: + return { + variant: 'success', + label: s__('Runners|online'), + tooltip: sprintf(I18N_ONLINE_RUNNER_DESCRIPTION, { + timeAgo: this.contactedAtTimeAgo, + }), + }; + case STATUS_OFFLINE: + return { + variant: 'muted', + label: s__('Runners|offline'), + tooltip: sprintf(I18N_OFFLINE_RUNNER_DESCRIPTION, { + timeAgo: this.contactedAtTimeAgo, + }), + }; + case STATUS_NOT_CONNECTED: + return { + variant: 'muted', + label: s__('Runners|not connected'), + tooltip: I18N_NOT_CONNECTED_RUNNER_DESCRIPTION, + }; + default: + return null; + } + }, + }, +}; +</script> +<template> + <gl-badge v-if="badge" v-gl-tooltip="badge.tooltip" :variant="badge.variant" v-bind="$attrs"> + {{ badge.label }} + </gl-badge> +</template> diff --git a/app/assets/javascripts/runner/components/runner_list.vue b/app/assets/javascripts/runner/components/runner_list.vue index a8796ae6a65..f8dbc469c22 100644 --- a/app/assets/javascripts/runner/components/runner_list.vue +++ b/app/assets/javascripts/runner/components/runner_list.vue @@ -5,7 +5,7 @@ import { __, s__ } from '~/locale'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import RunnerActionsCell from './cells/runner_actions_cell.vue'; import RunnerSummaryCell from './cells/runner_summary_cell.vue'; -import RunnerTypeCell from './cells/runner_type_cell.vue'; +import RunnerStatusCell from './cells/runner_status_cell.vue'; import RunnerTags from './runner_tags.vue'; const tableField = ({ key, label = '', width = 10 }) => { @@ -36,7 +36,7 @@ export default { RunnerActionsCell, RunnerSummaryCell, RunnerTags, - RunnerTypeCell, + RunnerStatusCell, }, directives: { GlTooltip: GlTooltipDirective, @@ -63,8 +63,8 @@ export default { }, }, fields: [ - tableField({ key: 'type', label: __('Type/State') }), - tableField({ key: 'summary', label: s__('Runners|Runner'), width: 30 }), + tableField({ key: 'status', label: s__('Runners|Status') }), + tableField({ key: 'summary', label: s__('Runners|Runner ID'), width: 30 }), tableField({ key: 'version', label: __('Version') }), tableField({ key: 'ipAddress', label: __('IP Address') }), tableField({ key: 'tagList', label: __('Tags'), width: 20 }), @@ -88,8 +88,8 @@ export default { <gl-skeleton-loader v-for="i in 4" :key="i" /> </template> - <template #cell(type)="{ item }"> - <runner-type-cell :runner="item" /> + <template #cell(status)="{ item }"> + <runner-status-cell :runner="item" /> </template> <template #cell(summary)="{ item, index }"> diff --git a/app/assets/javascripts/runner/components/runner_state_paused_badge.vue b/app/assets/javascripts/runner/components/runner_paused_badge.vue index d1e6fa05e4d..d1e6fa05e4d 100644 --- a/app/assets/javascripts/runner/components/runner_state_paused_badge.vue +++ b/app/assets/javascripts/runner/components/runner_paused_badge.vue diff --git a/app/assets/javascripts/runner/components/runner_state_locked_badge.vue b/app/assets/javascripts/runner/components/runner_state_locked_badge.vue deleted file mode 100644 index 458526010bc..00000000000 --- a/app/assets/javascripts/runner/components/runner_state_locked_badge.vue +++ /dev/null @@ -1,25 +0,0 @@ -<script> -import { GlBadge, GlTooltipDirective } from '@gitlab/ui'; -import { I18N_LOCKED_RUNNER_DESCRIPTION } from '../constants'; - -export default { - components: { - GlBadge, - }, - directives: { - GlTooltip: GlTooltipDirective, - }, - i18n: { - I18N_LOCKED_RUNNER_DESCRIPTION, - }, -}; -</script> -<template> - <gl-badge - v-gl-tooltip="$options.i18n.I18N_LOCKED_RUNNER_DESCRIPTION" - variant="warning" - v-bind="$attrs" - > - {{ s__('Runners|locked') }} - </gl-badge> -</template> diff --git a/app/assets/javascripts/runner/components/runner_type_alert.vue b/app/assets/javascripts/runner/components/runner_type_alert.vue index aa435aaa823..1400875a1d6 100644 --- a/app/assets/javascripts/runner/components/runner_type_alert.vue +++ b/app/assets/javascripts/runner/components/runner_type_alert.vue @@ -9,17 +9,14 @@ const ALERT_DATA = { message: s__( 'Runners|This runner is available to all groups and projects in your GitLab instance.', ), - variant: 'success', anchor: 'shared-runners', }, [GROUP_TYPE]: { message: s__('Runners|This runner is available to all projects and subgroups in a group.'), - variant: 'success', anchor: 'group-runners', }, [PROJECT_TYPE]: { message: s__('Runners|This runner is associated with one or more projects.'), - variant: 'info', anchor: 'specific-runners', }, }; @@ -50,7 +47,7 @@ export default { }; </script> <template> - <gl-alert v-if="alert" :variant="alert.variant" :dismissible="false"> + <gl-alert v-if="alert" variant="info" :dismissible="false"> {{ alert.message }} <gl-link :href="helpHref">{{ __('Learn more.') }}</gl-link> </gl-alert> diff --git a/app/assets/javascripts/runner/components/runner_type_badge.vue b/app/assets/javascripts/runner/components/runner_type_badge.vue index 1a61b80184b..b885dcefdcb 100644 --- a/app/assets/javascripts/runner/components/runner_type_badge.vue +++ b/app/assets/javascripts/runner/components/runner_type_badge.vue @@ -12,17 +12,14 @@ import { const BADGE_DATA = { [INSTANCE_TYPE]: { - variant: 'success', text: s__('Runners|shared'), tooltip: I18N_INSTANCE_RUNNER_DESCRIPTION, }, [GROUP_TYPE]: { - variant: 'success', text: s__('Runners|group'), tooltip: I18N_GROUP_RUNNER_DESCRIPTION, }, [PROJECT_TYPE]: { - variant: 'info', text: s__('Runners|specific'), tooltip: I18N_PROJECT_RUNNER_DESCRIPTION, }, @@ -53,7 +50,7 @@ export default { }; </script> <template> - <gl-badge v-if="badge" v-gl-tooltip="badge.tooltip" :variant="badge.variant" v-bind="$attrs"> + <gl-badge v-if="badge" v-gl-tooltip="badge.tooltip" variant="info" v-bind="$attrs"> {{ badge.text }} </gl-badge> </template> diff --git a/app/assets/javascripts/runner/constants.js b/app/assets/javascripts/runner/constants.js index b5f7ee67088..c0b256ec7fa 100644 --- a/app/assets/javascripts/runner/constants.js +++ b/app/assets/javascripts/runner/constants.js @@ -6,11 +6,24 @@ export const GROUP_RUNNER_COUNT_LIMIT = 1000; export const I18N_FETCH_ERROR = s__('Runners|Something went wrong while fetching runner data.'); export const I18N_DETAILS_TITLE = s__('Runners|Runner #%{runner_id}'); +// Type export const I18N_INSTANCE_RUNNER_DESCRIPTION = s__('Runners|Available to all projects'); export const I18N_GROUP_RUNNER_DESCRIPTION = s__( 'Runners|Available to all projects and subgroups in the group', ); export const I18N_PROJECT_RUNNER_DESCRIPTION = s__('Runners|Associated with one or more projects'); + +// Status +export const I18N_ONLINE_RUNNER_DESCRIPTION = s__( + 'Runners|Runner is online; last contact was %{timeAgo}', +); +export const I18N_OFFLINE_RUNNER_DESCRIPTION = s__( + 'Runners|No recent contact from this runner; last contact was %{timeAgo}', +); +export const I18N_NOT_CONNECTED_RUNNER_DESCRIPTION = s__( + 'Runners|This runner has never connected to this instance', +); + export const I18N_LOCKED_RUNNER_DESCRIPTION = s__('Runners|You cannot assign to other projects'); export const I18N_PAUSED_RUNNER_DESCRIPTION = s__('Runners|Not available to run jobs'); diff --git a/app/assets/javascripts/runner/graphql/runner_actions_update.mutation.graphql b/app/assets/javascripts/runner/graphql/runner_actions_update.mutation.graphql new file mode 100644 index 00000000000..547cc43907c --- /dev/null +++ b/app/assets/javascripts/runner/graphql/runner_actions_update.mutation.graphql @@ -0,0 +1,14 @@ +#import "~/runner/graphql/runner_node.fragment.graphql" + +# Mutation for updates within the runners list via action +# buttons (play, pause, ...), loads attributes shown in the +# runner list. + +mutation runnerActionsUpdate($input: RunnerUpdateInput!) { + runnerUpdate(input: $input) { + runner { + ...RunnerNode + } + errors + } +} diff --git a/app/assets/javascripts/runner/graphql/runner_node.fragment.graphql b/app/assets/javascripts/runner/graphql/runner_node.fragment.graphql index 0835e3c7c09..98f2dab26ca 100644 --- a/app/assets/javascripts/runner/graphql/runner_node.fragment.graphql +++ b/app/assets/javascripts/runner/graphql/runner_node.fragment.graphql @@ -10,4 +10,5 @@ fragment RunnerNode on CiRunner { locked tagList contactedAt + status } diff --git a/app/assets/javascripts/runner/graphql/runner_update.mutation.graphql b/app/assets/javascripts/runner/graphql/runner_update.mutation.graphql index dcc7fdf24f1..ea622fd4958 100644 --- a/app/assets/javascripts/runner/graphql/runner_update.mutation.graphql +++ b/app/assets/javascripts/runner/graphql/runner_update.mutation.graphql @@ -1,5 +1,8 @@ #import "ee_else_ce/runner/graphql/runner_details.fragment.graphql" +# Mutation for updates from the runner form, loads +# attributes shown in the runner details. + mutation runnerUpdate($input: RunnerUpdateInput!) { runnerUpdate(input: $input) { runner { diff --git a/app/graphql/mutations/issues/set_crm_contacts.rb b/app/graphql/mutations/issues/set_crm_contacts.rb new file mode 100644 index 00000000000..7a9e6237eaa --- /dev/null +++ b/app/graphql/mutations/issues/set_crm_contacts.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Mutations + module Issues + class SetCrmContacts < Base + graphql_name 'IssueSetCrmContacts' + + argument :crm_contact_ids, + [::Types::GlobalIDType[::CustomerRelations::Contact]], + required: true, + description: 'Customer relations contact IDs to set. Replaces existing contacts by default.' + + argument :operation_mode, + Types::MutationOperationModeEnum, + required: false, + description: 'Changes the operation mode. Defaults to REPLACE.' + + def resolve(project_path:, iid:, crm_contact_ids:, operation_mode: Types::MutationOperationModeEnum.enum[:replace]) + issue = authorized_find!(project_path: project_path, iid: iid) + project = issue.project + raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless Feature.enabled?(:customer_relations, project.group, default_enabled: :yaml) + + crm_contact_ids = crm_contact_ids.compact.map do |crm_contact_id| + raise Gitlab::Graphql::Errors::ArgumentError, "Contact #{crm_contact_id} is invalid." unless crm_contact_id.respond_to?(:model_id) + + crm_contact_id.model_id.to_i + end + + attribute_name = case operation_mode + when Types::MutationOperationModeEnum.enum[:append] + :add_crm_contact_ids + when Types::MutationOperationModeEnum.enum[:remove] + :remove_crm_contact_ids + else + :crm_contact_ids + end + + response = ::Issues::SetCrmContactsService.new(project: project, current_user: current_user, params: { attribute_name => crm_contact_ids }) + .execute(issue) + + { + issue: issue, + errors: response.errors + } + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 4104774c1de..7be995a40b2 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -49,6 +49,7 @@ module Types mount_mutation Mutations::Environments::CanaryIngress::Update mount_mutation Mutations::Issues::Create mount_mutation Mutations::Issues::SetAssignees + mount_mutation Mutations::Issues::SetCrmContacts mount_mutation Mutations::Issues::SetConfidential mount_mutation Mutations::Issues::SetLocked mount_mutation Mutations::Issues::SetDueDate diff --git a/app/models/customer_relations/issue_contact.rb b/app/models/customer_relations/issue_contact.rb index ba73b685906..98faf8d6644 100644 --- a/app/models/customer_relations/issue_contact.rb +++ b/app/models/customer_relations/issue_contact.rb @@ -15,6 +15,6 @@ class CustomerRelations::IssueContact < ApplicationRecord return unless issue&.project&.namespace_id return if contact.group_id == issue.project.namespace_id - errors.add(:base, _('The contact does not belong to the same group as the issue.')) + errors.add(:base, _('The contact does not belong to the same group as the issue')) end end diff --git a/app/models/group.rb b/app/models/group.rb index 4b87528d6e8..15eb2ee81a6 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -194,13 +194,8 @@ class Group < Namespace def ids_with_disabled_email(groups) inner_groups = Group.where('id = namespaces_with_emails_disabled.id') - inner_ancestors = if Feature.enabled?(:linear_group_ancestor_scopes, default_enabled: :yaml) - inner_groups.self_and_ancestors - else - Gitlab::ObjectHierarchy.new(inner_groups).base_and_ancestors - end - - inner_query = inner_ancestors + inner_query = inner_groups + .self_and_ancestors .where(emails_disabled: true) .select('1') .limit(1) diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb index 575e532c615..c9c13b29643 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -12,6 +12,9 @@ class IssuePolicy < IssuablePolicy @user && IssueCollection.new([@subject]).visible_to(@user).any? end + desc "User can read contacts belonging to the issue group" + condition(:can_read_crm_contacts, scope: :subject) { @user.can?(:read_crm_contact, @subject.project.group) } + desc "Issue is confidential" condition(:confidential, scope: :subject) { @subject.confidential? } @@ -77,6 +80,10 @@ class IssuePolicy < IssuablePolicy rule { ~persisted & can?(:create_issue) }.policy do enable :set_confidentiality end + + rule { can?(:set_issue_metadata) & can_read_crm_contacts }.policy do + enable :set_issue_crm_contacts + end end IssuePolicy.prepend_mod_with('IssuePolicy') diff --git a/app/services/issues/set_crm_contacts_service.rb b/app/services/issues/set_crm_contacts_service.rb new file mode 100644 index 00000000000..13fe30b5ac8 --- /dev/null +++ b/app/services/issues/set_crm_contacts_service.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +module Issues + class SetCrmContactsService < ::BaseProjectService + attr_accessor :issue, :errors + + MAX_ADDITIONAL_CONTACTS = 6 + + def execute(issue) + @issue = issue + @errors = [] + + return error_no_permissions unless allowed? + return error_invalid_params unless valid_params? + + determine_changes if params[:crm_contact_ids] + + return error_too_many if too_many? + + add_contacts if params[:add_crm_contact_ids] + remove_contacts if params[:remove_crm_contact_ids] + + if issue.valid? + ServiceResponse.success(payload: issue) + else + # The default error isn't very helpful: "Issue customer relations contacts is invalid" + issue.errors.delete(:issue_customer_relations_contacts) + issue.errors.add(:issue_customer_relations_contacts, errors.to_sentence) + ServiceResponse.error(payload: issue, message: issue.errors.full_messages) + end + end + + private + + def determine_changes + existing_contact_ids = issue.issue_customer_relations_contacts.map(&:contact_id) + params[:add_crm_contact_ids] = params[:crm_contact_ids] - existing_contact_ids + params[:remove_crm_contact_ids] = existing_contact_ids - params[:crm_contact_ids] + end + + def add_contacts + params[:add_crm_contact_ids].uniq.each do |contact_id| + issue_contact = issue.issue_customer_relations_contacts.create(contact_id: contact_id) + + unless issue_contact.persisted? + # The validation ensures that the id exists and the user has permission + errors << "#{contact_id}: The resource that you are attempting to access does not exist or you don't have permission to perform this action" + end + end + end + + def remove_contacts + issue.issue_customer_relations_contacts + .where(contact_id: params[:remove_crm_contact_ids]) # rubocop: disable CodeReuse/ActiveRecord + .delete_all + end + + def allowed? + current_user&.can?(:set_issue_crm_contacts, issue) + end + + def valid_params? + set_present? ^ add_or_remove_present? + end + + def set_present? + params[:crm_contact_ids].present? + end + + def add_or_remove_present? + params[:add_crm_contact_ids].present? || params[:remove_crm_contact_ids].present? + end + + def too_many? + params[:add_crm_contact_ids] && params[:add_crm_contact_ids].length > MAX_ADDITIONAL_CONTACTS + end + + def error_no_permissions + ServiceResponse.error(message: ['You have insufficient permissions to set customer relations contacts for this issue']) + end + + def error_invalid_params + ServiceResponse.error(message: ['You cannot combine crm_contact_ids with add_crm_contact_ids or remove_crm_contact_ids']) + end + + def error_too_many + ServiceResponse.error(payload: issue, message: ["You can only add up to #{MAX_ADDITIONAL_CONTACTS} contacts at one time"]) + end + end +end diff --git a/app/services/projects/container_repository/cleanup_tags_service.rb b/app/services/projects/container_repository/cleanup_tags_service.rb index b25205d2894..1a788abac12 100644 --- a/app/services/projects/container_repository/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_service.rb @@ -146,8 +146,7 @@ module Projects def caching_enabled? container_expiration_policy && - older_than.present? && - Feature.enabled?(:container_registry_expiration_policies_caching, @project) + older_than.present? end def throttling_enabled? diff --git a/app/views/projects/product_analytics/_links.html.haml b/app/views/projects/product_analytics/_links.html.haml index 0797c5baf91..6e5667e2644 100644 --- a/app/views/projects/product_analytics/_links.html.haml +++ b/app/views/projects/product_analytics/_links.html.haml @@ -1,10 +1,5 @@ -.mb-3 - %ul.nav-links - = nav_link(path: 'product_analytics#index') do - = link_to _('Events'), project_product_analytics_path(@project) - = nav_link(path: 'product_analytics#graphs') do - = link_to 'Graphs', graphs_project_product_analytics_path(@project) - = nav_link(path: 'product_analytics#test') do - = link_to _('Test'), test_project_product_analytics_path(@project) - = nav_link(path: 'product_analytics#setup') do - = link_to _('Setup'), setup_project_product_analytics_path(@project) += gl_tabs_nav({ class: 'mb-3'}) do + = gl_tab_link_to _('Events'), project_product_analytics_path(@project) + = gl_tab_link_to _('Graphs'), graphs_project_product_analytics_path(@project) + = gl_tab_link_to _('Test'), test_project_product_analytics_path(@project) + = gl_tab_link_to _('Setup'), setup_project_product_analytics_path(@project) diff --git a/config/feature_flags/development/linear_group_ancestor_scopes.yml b/config/feature_flags/development/api_v3_commits_skip_diff_files.yml index f23399c1e6f..d690acf6c1d 100644 --- a/config/feature_flags/development/linear_group_ancestor_scopes.yml +++ b/config/feature_flags/development/api_v3_commits_skip_diff_files.yml @@ -1,8 +1,8 @@ --- -name: linear_group_ancestor_scopes -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70495 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/341115 -milestone: '14.4' +name: api_v3_commits_skip_diff_files +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67647 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344617 +milestone: '14.5' type: development -group: group::access +group: group::integrations default_enabled: false diff --git a/config/feature_flags/development/ci_new_artifact_file_reader.yml b/config/feature_flags/development/ci_new_artifact_file_reader.yml deleted file mode 100644 index d475f3f370d..00000000000 --- a/config/feature_flags/development/ci_new_artifact_file_reader.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_new_artifact_file_reader -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46552 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/273755 -milestone: '13.6' -type: development -group: group::pipeline authoring -default_enabled: true diff --git a/config/feature_flags/development/container_registry_expiration_policies_caching.yml b/config/feature_flags/development/container_registry_expiration_policies_caching.yml deleted file mode 100644 index 6e8b0efe94d..00000000000 --- a/config/feature_flags/development/container_registry_expiration_policies_caching.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: container_registry_expiration_policies_caching -introduced_by_url: -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340606 -milestone: '14.3' -type: development -group: group::package -default_enabled: false diff --git a/config/initializers/0_acts_as_taggable.rb b/config/initializers/1_acts_as_taggable.rb index 9a92b8f2d18..59412aef755 100644 --- a/config/initializers/0_acts_as_taggable.rb +++ b/config/initializers/1_acts_as_taggable.rb @@ -9,3 +9,9 @@ ActsAsTaggableOn.tags_counter = false # validate that counter cache is disabled raise "Counter cache is not disabled" if ActsAsTaggableOn::Tagging.reflections["tag"].options[:counter_cache] + +# Redirects retrieve_connection to use Ci::ApplicationRecord's connection +[::ActsAsTaggableOn::Tag, ::ActsAsTaggableOn::Tagging].each do |model| + model.connection_specification_name = Ci::ApplicationRecord.connection_specification_name + model.singleton_class.delegate :connection, :sticking, to: '::Ci::ApplicationRecord' +end diff --git a/config/initializers/database_query_analyzers.rb b/config/initializers/database_query_analyzers.rb new file mode 100644 index 00000000000..a29cd6552ff --- /dev/null +++ b/config/initializers/database_query_analyzers.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +# Currently we register validator only for `dev` or `test` environment +Gitlab::Database::QueryAnalyzer.new.hook! if Gitlab.dev_or_test_env? diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 01f1a98cd2a..c47b1151f6a 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2820,6 +2820,28 @@ Input type: `IssueSetConfidentialInput` | <a id="mutationissuesetconfidentialerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | <a id="mutationissuesetconfidentialissue"></a>`issue` | [`Issue`](#issue) | Issue after mutation. | +### `Mutation.issueSetCrmContacts` + +Input type: `IssueSetCrmContactsInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationissuesetcrmcontactsclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationissuesetcrmcontactscrmcontactids"></a>`crmContactIds` | [`[CustomerRelationsContactID!]!`](#customerrelationscontactid) | Customer relations contact IDs to set. Replaces existing contacts by default. | +| <a id="mutationissuesetcrmcontactsiid"></a>`iid` | [`String!`](#string) | IID of the issue to mutate. | +| <a id="mutationissuesetcrmcontactsoperationmode"></a>`operationMode` | [`MutationOperationMode`](#mutationoperationmode) | Changes the operation mode. Defaults to REPLACE. | +| <a id="mutationissuesetcrmcontactsprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the issue to mutate is in. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationissuesetcrmcontactsclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationissuesetcrmcontactserrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +| <a id="mutationissuesetcrmcontactsissue"></a>`issue` | [`Issue`](#issue) | Issue after mutation. | + ### `Mutation.issueSetDueDate` Input type: `IssueSetDueDateInput` diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index fefd34e855d..e6a5625a827 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -175,13 +175,13 @@ is similar to [`rules:` defined in jobs](#rules). You can use the [`workflow:rules` templates](#workflowrules-templates) to import a preconfigured `workflow: rules` entry. -`workflow: rules` accepts these keywords: +`workflow: rules` accepts some of the same keywords as [`rules`](#rules): -- [`if`](#rulesif): Check this rule to determine when to run a pipeline. -- [`when`](#when): Specify what to do when the `if` rule evaluates to true. - - To run a pipeline, set to `always`. - - To prevent pipelines from running, set to `never`. -- [`variables`](#workflowrulesvariables): If not defined, uses the [variables defined elsewhere](#variables). +- [`rules: if`](#rulesif). +- [`rules: changes`](#ruleschanges). +- [`rules: exists`](#rulesexists). +- [`when`](#when), can only be `always` or `never` when used with `workflow`. +- [`variables`](#workflowrulesvariables). When no rules evaluate to true, the pipeline does not run. diff --git a/lib/api/github/entities.rb b/lib/api/github/entities.rb index fe228c9a2d2..125985f0e23 100644 --- a/lib/api/github/entities.rb +++ b/lib/api/github/entities.rb @@ -59,8 +59,8 @@ module API expose :parents do |commit| commit.parent_ids.map { |id| { sha: id } } end - expose :files do |commit| - commit.diffs.diff_files.flat_map do |diff| + expose :files do |_commit, options| + options[:diff_files].flat_map do |diff| additions = diff.added_lines deletions = diff.removed_lines diff --git a/lib/api/v3/github.rb b/lib/api/v3/github.rb index 310054c298a..3b834f5e7e7 100644 --- a/lib/api/v3/github.rb +++ b/lib/api/v3/github.rb @@ -20,6 +20,9 @@ module API # Jira Server user agent format: Jira DVCS Connector/version JIRA_DVCS_CLOUD_USER_AGENT = 'Jira DVCS Connector Vertigo' + GITALY_TIMEOUT_CACHE_KEY = 'api:v3:Gitaly-timeout-cache-key' + GITALY_TIMEOUT_CACHE_EXPIRY = 1.day + include PaginationParams feature_category :integrations @@ -93,6 +96,32 @@ module API notes.select { |n| n.readable_by?(current_user) } end # rubocop: enable CodeReuse/ActiveRecord + + # Returns an empty Array instead of the Commit diff files for a period + # of time after a Gitaly timeout, to mitigate frequent Gitaly timeouts + # for some Commit diffs. + def diff_files(commit) + return commit.diffs.diff_files unless Feature.enabled?(:api_v3_commits_skip_diff_files, commit.project) + + cache_key = [ + GITALY_TIMEOUT_CACHE_KEY, + commit.project.id, + commit.cache_key + ].join(':') + + return [] if Rails.cache.read(cache_key).present? + + begin + commit.diffs.diff_files + rescue GRPC::DeadlineExceeded => error + # Gitaly fails to load diffs consistently for some commits. The other information + # is still valuable for Jira. So we skip the loading and respond with a 200 excluding diffs + # Remove this when https://gitlab.com/gitlab-org/gitaly/-/issues/3741 is fixed. + Rails.cache.write(cache_key, 1, expires_in: GITALY_TIMEOUT_CACHE_EXPIRY) + Gitlab::ErrorTracking.track_exception(error) + [] + end + end end resource :orgs do @@ -228,10 +257,9 @@ module API user_project = find_project_with_access(params) commit = user_project.commit(params[:sha]) - not_found! 'Commit' unless commit - present commit, with: ::API::Github::Entities::RepoCommit + present commit, with: ::API::Github::Entities::RepoCommit, diff_files: diff_files(commit) end end end diff --git a/lib/gitlab/ci/artifact_file_reader.rb b/lib/gitlab/ci/artifact_file_reader.rb index 3cfed8e5e2c..b0fad026ec5 100644 --- a/lib/gitlab/ci/artifact_file_reader.rb +++ b/lib/gitlab/ci/artifact_file_reader.rb @@ -45,14 +45,6 @@ module Gitlab end def read_zip_file!(file_path) - if ::Feature.enabled?(:ci_new_artifact_file_reader, job.project, default_enabled: :yaml) - read_with_new_artifact_file_reader(file_path) - else - read_with_legacy_artifact_file_reader(file_path) - end - end - - def read_with_new_artifact_file_reader(file_path) job.artifacts_file.use_open_file do |file| zip_file = Zip::File.new(file, false, true) entry = zip_file.find_entry(file_path) @@ -69,25 +61,6 @@ module Gitlab end end - def read_with_legacy_artifact_file_reader(file_path) - job.artifacts_file.use_file do |archive_path| - Zip::File.open(archive_path) do |zip_file| - entry = zip_file.find_entry(file_path) - unless entry - raise Error, "Path `#{file_path}` does not exist inside the `#{job.name}` artifacts archive!" - end - - if entry.name_is_directory? - raise Error, "Path `#{file_path}` was expected to be a file but it was a directory!" - end - - zip_file.get_input_stream(entry) do |is| - is.read - end - end - end - end - def max_archive_size_in_mb ActiveSupport::NumberHelper.number_to_human_size(MAX_ARCHIVE_SIZE) end diff --git a/lib/gitlab/database/gitlab_schema.rb b/lib/gitlab/database/gitlab_schema.rb index 7969f6cda40..14807494a79 100644 --- a/lib/gitlab/database/gitlab_schema.rb +++ b/lib/gitlab/database/gitlab_schema.rb @@ -8,17 +8,84 @@ # - gitlab_shared - defines a set of tables that are found on all databases (data accessed is dependent on connection) # - gitlab_main / gitlab_ci - defines a set of tables that can only exist on a given database # +# Tables for the purpose of tests should be prefixed with `_test_my_table_name` module Gitlab module Database module GitlabSchema + # These tables are deleted/renamed, but still referenced by migrations. + # This is needed for now, but should be removed in the future + DELETED_TABLES = { + # main tables + 'alerts_service_data' => :gitlab_main, + 'analytics_devops_adoption_segment_selections' => :gitlab_main, + 'analytics_repository_file_commits' => :gitlab_main, + 'analytics_repository_file_edits' => :gitlab_main, + 'analytics_repository_files' => :gitlab_main, + 'audit_events_archived' => :gitlab_main, + 'backup_labels' => :gitlab_main, + 'clusters_applications_fluentd' => :gitlab_main, + 'forked_project_links' => :gitlab_main, + 'issue_milestones' => :gitlab_main, + 'merge_request_milestones' => :gitlab_main, + 'namespace_onboarding_actions' => :gitlab_main, + 'services' => :gitlab_main, + 'terraform_state_registry' => :gitlab_main, + 'tmp_fingerprint_sha256_migration' => :gitlab_main, # used by lib/gitlab/background_migration/migrate_fingerprint_sha256_within_keys.rb + 'web_hook_logs_archived' => :gitlab_main, + 'vulnerability_export_registry' => :gitlab_main, + 'vulnerability_finding_fingerprints' => :gitlab_main, + 'vulnerability_export_verification_status' => :gitlab_main, + + # CI tables + 'ci_build_trace_sections' => :gitlab_ci, + 'ci_build_trace_section_names' => :gitlab_ci, + 'ci_daily_report_results' => :gitlab_ci, + 'ci_test_cases' => :gitlab_ci, + 'ci_test_case_failures' => :gitlab_ci, + + # leftovers from early implementation of partitioning + 'audit_events_part_5fc467ac26' => :gitlab_main, + 'web_hook_logs_part_0c5294f417' => :gitlab_main + }.freeze + def self.table_schemas(tables) tables.map { |table| table_schema(table) }.to_set end def self.table_schema(name) + schema_name, table_name = name.split('.', 2) # Strip schema name like: `public.` + + # Most of names do not have schemas, ensure that this is table + unless table_name + table_name = schema_name + schema_name = nil + end + + # strip partition number of a form `loose_foreign_keys_deleted_records_1` + table_name.gsub!(/_[0-9]+$/, '') + + # Tables that are properly mapped + if gitlab_schema = tables_to_schema[table_name] + return gitlab_schema + end + + # Tables that are deleted, but we still need to reference them + if gitlab_schema = DELETED_TABLES[table_name] + return gitlab_schema + end + + # All tables from `information_schema.` are `:gitlab_shared` + return :gitlab_shared if schema_name == 'information_schema' + + # All tables that start with `_test_` are shared and ignored + return :gitlab_shared if table_name.start_with?('_test_') + + # All `pg_` tables are marked as `shared` + return :gitlab_shared if table_name.start_with?('pg_') + # When undefined it's best to return a unique name so that we don't incorrectly assume that 2 undefined schemas belong on the same database - tables_to_schema[name] || :"undefined_#{name}" + :"undefined_#{table_name}" end def self.tables_to_schema diff --git a/lib/gitlab/database/load_balancing/configuration.rb b/lib/gitlab/database/load_balancing/configuration.rb index 7814083a527..da313361073 100644 --- a/lib/gitlab/database/load_balancing/configuration.rb +++ b/lib/gitlab/database/load_balancing/configuration.rb @@ -77,6 +77,10 @@ module Gitlab (@primary_model || @model).connection_specification_name end + def primary_db_config + (@primary_model || @model).connection_db_config + end + def replica_db_config @model.connection_db_config end diff --git a/lib/gitlab/database/query_analyzer.rb b/lib/gitlab/database/query_analyzer.rb new file mode 100644 index 00000000000..830ad1383c0 --- /dev/null +++ b/lib/gitlab/database/query_analyzer.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Gitlab + module Database + # The purpose of this class is to implement a various query analyzers based on `pg_query` + # And process them all via `Gitlab::Database::QueryAnalyzers::*` + class QueryAnalyzer + ANALYZERS = [].freeze + + Parsed = Struct.new( + :sql, :connection, :pg + ) + + def hook! + @subscriber = ActiveSupport::Notifications.subscribe('sql.active_record') do |event| + process_sql(event.payload[:sql], event.payload[:connection]) + end + end + + private + + def process_sql(sql, connection) + analyzers = enabled_analyzers(connection) + return unless analyzers.any? + + parsed = parse(sql, connection) + return unless parsed + + analyzers.each do |analyzer| + analyzer.analyze(parsed) + rescue => e # rubocop:disable Style/RescueStandardError + # We catch all standard errors to prevent validation errors to introduce fatal errors in production + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) + end + end + + def enabled_analyzers(connection) + ANALYZERS.select do |analyzer| + analyzer.enabled?(connection) + rescue StandardError => e # rubocop:disable Style/RescueStandardError + # We catch all standard errors to prevent validation errors to introduce fatal errors in production + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) + end + end + + def parse(sql, connection) + parsed = PgQuery.parse(sql) + return unless parsed + + normalized = PgQuery.normalize(sql) + Parsed.new(normalized, connection, parsed) + rescue PgQuery::ParseError => e + # Ignore PgQuery parse errors (due to depth limit or other reasons) + Gitlab::ErrorTracking.track_exception(e) + + nil + end + end + end +end diff --git a/lib/gitlab/database/query_analyzers/base.rb b/lib/gitlab/database/query_analyzers/base.rb new file mode 100644 index 00000000000..e1be889c803 --- /dev/null +++ b/lib/gitlab/database/query_analyzers/base.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module QueryAnalyzers + class Base + def self.enabled?(connection) + raise NotImplementedError + end + + def self.analyze(parsed) + raise NotImplementedError + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ca8befc9ba4..4752b246616 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16109,6 +16109,9 @@ msgstr "" msgid "GraphViewType|Stage" msgstr "" +msgid "Graphs" +msgstr "" + msgid "Gravatar" msgstr "" @@ -29691,6 +29694,9 @@ msgstr "" msgid "Runners|New runner, has not connected yet" msgstr "" +msgid "Runners|No recent contact from this runner; last contact was %{timeAgo}" +msgstr "" + msgid "Runners|Not available to run jobs" msgstr "" @@ -29742,6 +29748,9 @@ msgstr "" msgid "Runners|Runner #%{runner_id}" msgstr "" +msgid "Runners|Runner ID" +msgstr "" + msgid "Runners|Runner assigned to project." msgstr "" @@ -29751,6 +29760,9 @@ msgstr "" msgid "Runners|Runner is online, last contact was %{runner_contact} ago" msgstr "" +msgid "Runners|Runner is online; last contact was %{timeAgo}" +msgstr "" + msgid "Runners|Runner is paused, last contact was %{runner_contact} ago" msgstr "" @@ -29781,12 +29793,18 @@ msgstr "" msgid "Runners|Something went wrong while fetching the tags suggestions" msgstr "" +msgid "Runners|Status" +msgstr "" + msgid "Runners|Stop the runner from accepting new jobs." msgstr "" msgid "Runners|Tags" msgstr "" +msgid "Runners|This runner has never connected to this instance" +msgstr "" + msgid "Runners|This runner is associated with one or more projects." msgstr "" @@ -29853,6 +29871,15 @@ msgstr "" msgid "Runners|locked" msgstr "" +msgid "Runners|not connected" +msgstr "" + +msgid "Runners|offline" +msgstr "" + +msgid "Runners|online" +msgstr "" + msgid "Runners|paused" msgstr "" @@ -34136,7 +34163,7 @@ msgstr "" msgid "The connection will time out after %{timeout}. For repositories that take longer, use a clone/push combination." msgstr "" -msgid "The contact does not belong to the same group as the issue." +msgid "The contact does not belong to the same group as the issue" msgstr "" msgid "The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository." diff --git a/qa/qa/page/group/settings/package_registries.rb b/qa/qa/page/group/settings/package_registries.rb index 5c93c0d6222..1e9e088ac54 100644 --- a/qa/qa/page/group/settings/package_registries.rb +++ b/qa/qa/page/group/settings/package_registries.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true - module QA module Page module Group @@ -20,22 +19,33 @@ module QA def set_allow_duplicates_disabled expand_content :package_registry_settings_content do - click_element(:allow_duplicates_toggle) if duplicates_enabled? + click_on_allow_duplicates_button if duplicates_enabled? end end def set_allow_duplicates_enabled expand_content :package_registry_settings_content do - click_element(:allow_duplicates_toggle) if duplicates_disabled? + click_on_allow_duplicates_button unless duplicates_enabled? + end + end + + def click_on_allow_duplicates_button + with_allow_duplicates_button do |button| + button.click end end def duplicates_enabled? - has_element?(:allow_duplicates_label, text: 'Allow duplicates') + with_allow_duplicates_button do |button| + button[:class].include?('is-checked') + end end - def duplicates_disabled? - has_element?(:allow_duplicates_label, text: 'Do not allow duplicates') + def with_allow_duplicates_button + within_element :allow_duplicates_toggle do + toggle = find('button.gl-toggle') + yield(toggle) + end end def has_dependency_proxy_enabled? diff --git a/spec/frontend/runner/components/cells/runner_actions_cell_spec.js b/spec/frontend/runner/components/cells/runner_actions_cell_spec.js index 5aa3879ac3e..2874bdbe280 100644 --- a/spec/frontend/runner/components/cells/runner_actions_cell_spec.js +++ b/spec/frontend/runner/components/cells/runner_actions_cell_spec.js @@ -8,12 +8,11 @@ import RunnerActionCell from '~/runner/components/cells/runner_actions_cell.vue' import getGroupRunnersQuery from '~/runner/graphql/get_group_runners.query.graphql'; import getRunnersQuery from '~/runner/graphql/get_runners.query.graphql'; import runnerDeleteMutation from '~/runner/graphql/runner_delete.mutation.graphql'; -import runnerUpdateMutation from '~/runner/graphql/runner_update.mutation.graphql'; +import runnerActionsUpdateMutation from '~/runner/graphql/runner_actions_update.mutation.graphql'; import { captureException } from '~/runner/sentry_utils'; -import { runnersData, runnerData } from '../../mock_data'; +import { runnersData } from '../../mock_data'; const mockRunner = runnersData.data.runners.nodes[0]; -const mockRunnerDetails = runnerData.data.runner; const getRunnersQueryName = getRunnersQuery.definitions[0].name.value; const getGroupRunnersQueryName = getGroupRunnersQuery.definitions[0].name.value; @@ -27,7 +26,7 @@ jest.mock('~/runner/sentry_utils'); describe('RunnerTypeCell', () => { let wrapper; const runnerDeleteMutationHandler = jest.fn(); - const runnerUpdateMutationHandler = jest.fn(); + const runnerActionsUpdateMutationHandler = jest.fn(); const findEditBtn = () => wrapper.findByTestId('edit-runner'); const findToggleActiveBtn = () => wrapper.findByTestId('toggle-active-runner'); @@ -46,7 +45,7 @@ describe('RunnerTypeCell', () => { localVue, apolloProvider: createMockApollo([ [runnerDeleteMutation, runnerDeleteMutationHandler], - [runnerUpdateMutation, runnerUpdateMutationHandler], + [runnerActionsUpdateMutation, runnerActionsUpdateMutationHandler], ]), ...options, }), @@ -62,10 +61,10 @@ describe('RunnerTypeCell', () => { }, }); - runnerUpdateMutationHandler.mockResolvedValue({ + runnerActionsUpdateMutationHandler.mockResolvedValue({ data: { runnerUpdate: { - runner: mockRunnerDetails, + runner: mockRunner, errors: [], }, }, @@ -74,7 +73,7 @@ describe('RunnerTypeCell', () => { afterEach(() => { runnerDeleteMutationHandler.mockReset(); - runnerUpdateMutationHandler.mockReset(); + runnerActionsUpdateMutationHandler.mockReset(); wrapper.destroy(); }); @@ -116,12 +115,12 @@ describe('RunnerTypeCell', () => { describe(`When clicking on the ${icon} button`, () => { it(`The apollo mutation to set active to ${newActiveValue} is called`, async () => { - expect(runnerUpdateMutationHandler).toHaveBeenCalledTimes(0); + expect(runnerActionsUpdateMutationHandler).toHaveBeenCalledTimes(0); await findToggleActiveBtn().vm.$emit('click'); - expect(runnerUpdateMutationHandler).toHaveBeenCalledTimes(1); - expect(runnerUpdateMutationHandler).toHaveBeenCalledWith({ + expect(runnerActionsUpdateMutationHandler).toHaveBeenCalledTimes(1); + expect(runnerActionsUpdateMutationHandler).toHaveBeenCalledWith({ input: { id: mockRunner.id, active: newActiveValue, @@ -145,7 +144,7 @@ describe('RunnerTypeCell', () => { const mockErrorMsg = 'Update error!'; beforeEach(async () => { - runnerUpdateMutationHandler.mockRejectedValueOnce(new Error(mockErrorMsg)); + runnerActionsUpdateMutationHandler.mockRejectedValueOnce(new Error(mockErrorMsg)); await findToggleActiveBtn().vm.$emit('click'); }); @@ -167,10 +166,10 @@ describe('RunnerTypeCell', () => { const mockErrorMsg2 = 'User not allowed!'; beforeEach(async () => { - runnerUpdateMutationHandler.mockResolvedValue({ + runnerActionsUpdateMutationHandler.mockResolvedValue({ data: { runnerUpdate: { - runner: runnerData.data.runner, + runner: mockRunner, errors: [mockErrorMsg, mockErrorMsg2], }, }, diff --git a/spec/frontend/runner/components/cells/runner_status_cell_spec.js b/spec/frontend/runner/components/cells/runner_status_cell_spec.js new file mode 100644 index 00000000000..20a1cdf7236 --- /dev/null +++ b/spec/frontend/runner/components/cells/runner_status_cell_spec.js @@ -0,0 +1,69 @@ +import { GlBadge } from '@gitlab/ui'; +import { mount } from '@vue/test-utils'; +import RunnerStatusCell from '~/runner/components/cells/runner_status_cell.vue'; +import { INSTANCE_TYPE, STATUS_ONLINE, STATUS_OFFLINE } from '~/runner/constants'; + +describe('RunnerTypeCell', () => { + let wrapper; + + const findBadgeAt = (i) => wrapper.findAllComponents(GlBadge).at(i); + + const createComponent = ({ runner = {} } = {}) => { + wrapper = mount(RunnerStatusCell, { + propsData: { + runner: { + runnerType: INSTANCE_TYPE, + active: true, + status: STATUS_ONLINE, + ...runner, + }, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + it('Displays online status', () => { + createComponent(); + + expect(wrapper.text()).toMatchInterpolatedText('online'); + expect(findBadgeAt(0).text()).toBe('online'); + }); + + it('Displays offline status', () => { + createComponent({ + runner: { + status: STATUS_OFFLINE, + }, + }); + + expect(wrapper.text()).toMatchInterpolatedText('offline'); + expect(findBadgeAt(0).text()).toBe('offline'); + }); + + it('Displays paused status', () => { + createComponent({ + runner: { + active: false, + status: STATUS_ONLINE, + }, + }); + + expect(wrapper.text()).toMatchInterpolatedText('online paused'); + + expect(findBadgeAt(0).text()).toBe('online'); + expect(findBadgeAt(1).text()).toBe('paused'); + }); + + it('Is empty when data is missing', () => { + createComponent({ + runner: { + status: null, + }, + }); + + expect(wrapper.text()).toBe(''); + }); +}); diff --git a/spec/frontend/runner/components/cells/runner_summary_cell_spec.js b/spec/frontend/runner/components/cells/runner_summary_cell_spec.js index 1c9282e0acd..b6d957d27ea 100644 --- a/spec/frontend/runner/components/cells/runner_summary_cell_spec.js +++ b/spec/frontend/runner/components/cells/runner_summary_cell_spec.js @@ -1,5 +1,6 @@ -import { mount } from '@vue/test-utils'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; import RunnerSummaryCell from '~/runner/components/cells/runner_summary_cell.vue'; +import { INSTANCE_TYPE, PROJECT_TYPE } from '~/runner/constants'; const mockId = '1'; const mockShortSha = '2P6oDVDm'; @@ -8,13 +9,17 @@ const mockDescription = 'runner-1'; describe('RunnerTypeCell', () => { let wrapper; - const createComponent = (options) => { - wrapper = mount(RunnerSummaryCell, { + const findLockIcon = () => wrapper.findByTestId('lock-icon'); + + const createComponent = (runner, options) => { + wrapper = mountExtended(RunnerSummaryCell, { propsData: { runner: { id: `gid://gitlab/Ci::Runner/${mockId}`, shortSha: mockShortSha, description: mockDescription, + runnerType: INSTANCE_TYPE, + ...runner, }, }, ...options, @@ -33,6 +38,23 @@ describe('RunnerTypeCell', () => { expect(wrapper.text()).toContain(`#${mockId} (${mockShortSha})`); }); + it('Displays the runner type', () => { + expect(wrapper.text()).toContain('shared'); + }); + + it('Does not display the locked icon', () => { + expect(findLockIcon().exists()).toBe(false); + }); + + it('Displays the locked icon for locked runners', () => { + createComponent({ + runnerType: PROJECT_TYPE, + locked: true, + }); + + expect(findLockIcon().exists()).toBe(true); + }); + it('Displays the runner description', () => { expect(wrapper.text()).toContain(mockDescription); }); @@ -40,11 +62,14 @@ describe('RunnerTypeCell', () => { it('Displays a custom slot', () => { const slotContent = 'My custom runner summary'; - createComponent({ - slots: { - 'runner-name': slotContent, + createComponent( + {}, + { + slots: { + 'runner-name': slotContent, + }, }, - }); + ); expect(wrapper.text()).toContain(slotContent); }); diff --git a/spec/frontend/runner/components/cells/runner_type_cell_spec.js b/spec/frontend/runner/components/cells/runner_type_cell_spec.js deleted file mode 100644 index 48958a282fc..00000000000 --- a/spec/frontend/runner/components/cells/runner_type_cell_spec.js +++ /dev/null @@ -1,48 +0,0 @@ -import { GlBadge } from '@gitlab/ui'; -import { mount } from '@vue/test-utils'; -import RunnerTypeCell from '~/runner/components/cells/runner_type_cell.vue'; -import { INSTANCE_TYPE } from '~/runner/constants'; - -describe('RunnerTypeCell', () => { - let wrapper; - - const findBadges = () => wrapper.findAllComponents(GlBadge); - - const createComponent = ({ runner = {} } = {}) => { - wrapper = mount(RunnerTypeCell, { - propsData: { - runner: { - runnerType: INSTANCE_TYPE, - active: true, - locked: false, - ...runner, - }, - }, - }); - }; - - afterEach(() => { - wrapper.destroy(); - }); - - it('Displays the runner type', () => { - createComponent(); - - expect(findBadges()).toHaveLength(1); - expect(findBadges().at(0).text()).toBe('shared'); - }); - - it('Displays locked and paused states', () => { - createComponent({ - runner: { - active: false, - locked: true, - }, - }); - - expect(findBadges()).toHaveLength(3); - expect(findBadges().at(0).text()).toBe('shared'); - expect(findBadges().at(1).text()).toBe('locked'); - expect(findBadges().at(2).text()).toBe('paused'); - }); -}); diff --git a/spec/frontend/runner/components/runner_contacted_state_badge_spec.js b/spec/frontend/runner/components/runner_contacted_state_badge_spec.js new file mode 100644 index 00000000000..57a27f39826 --- /dev/null +++ b/spec/frontend/runner/components/runner_contacted_state_badge_spec.js @@ -0,0 +1,86 @@ +import { GlBadge } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import RunnerContactedStateBadge from '~/runner/components/runner_contacted_state_badge.vue'; +import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; +import { STATUS_ONLINE, STATUS_OFFLINE, STATUS_NOT_CONNECTED } from '~/runner/constants'; + +describe('RunnerTypeBadge', () => { + let wrapper; + + const findBadge = () => wrapper.findComponent(GlBadge); + const getTooltip = () => getBinding(findBadge().element, 'gl-tooltip'); + + const createComponent = ({ runner = {} } = {}) => { + wrapper = shallowMount(RunnerContactedStateBadge, { + propsData: { + runner: { + contactedAt: '2021-01-01T00:00:00Z', + status: STATUS_ONLINE, + ...runner, + }, + }, + directives: { + GlTooltip: createMockDirective(), + }, + }); + }; + + beforeEach(() => { + jest.useFakeTimers('modern'); + }); + + afterEach(() => { + jest.useFakeTimers('legacy'); + + wrapper.destroy(); + }); + + it('renders online state', () => { + jest.setSystemTime(new Date('2021-01-01T00:01:00Z')); + + createComponent(); + + expect(wrapper.text()).toBe('online'); + expect(findBadge().props('variant')).toBe('success'); + expect(getTooltip().value).toBe('Runner is online; last contact was 1 minute ago'); + }); + + it('renders offline state', () => { + jest.setSystemTime(new Date('2021-01-02T00:00:00Z')); + + createComponent({ + runner: { + status: STATUS_OFFLINE, + }, + }); + + expect(wrapper.text()).toBe('offline'); + expect(findBadge().props('variant')).toBe('muted'); + expect(getTooltip().value).toBe( + 'No recent contact from this runner; last contact was 1 day ago', + ); + }); + + it('renders not connected state', () => { + createComponent({ + runner: { + contactedAt: null, + status: STATUS_NOT_CONNECTED, + }, + }); + + expect(wrapper.text()).toBe('not connected'); + expect(findBadge().props('variant')).toBe('muted'); + expect(getTooltip().value).toMatch('This runner has never connected'); + }); + + it('does not fail when data is missing', () => { + createComponent({ + runner: { + status: null, + }, + }); + + expect(wrapper.text()).toBe(''); + }); +}); diff --git a/spec/frontend/runner/components/runner_list_spec.js b/spec/frontend/runner/components/runner_list_spec.js index a3814c1277a..986e55a2132 100644 --- a/spec/frontend/runner/components/runner_list_spec.js +++ b/spec/frontend/runner/components/runner_list_spec.js @@ -42,8 +42,8 @@ describe('RunnerList', () => { const headerLabels = findHeaders().wrappers.map((w) => w.text()); expect(headerLabels).toEqual([ - 'Type/State', - 'Runner', + 'Status', + 'Runner ID', 'Version', 'IP Address', 'Tags', @@ -62,7 +62,7 @@ describe('RunnerList', () => { const { id, description, version, ipAddress, shortSha } = mockRunners[0]; // Badges - expect(findCell({ fieldKey: 'type' }).text()).toMatchInterpolatedText('specific paused'); + expect(findCell({ fieldKey: 'status' }).text()).toMatchInterpolatedText('not connected paused'); // Runner summary expect(findCell({ fieldKey: 'summary' }).text()).toContain( diff --git a/spec/frontend/runner/components/runner_state_paused_badge_spec.js b/spec/frontend/runner/components/runner_paused_badge_spec.js index 8df56d6e3f3..18cfcfae864 100644 --- a/spec/frontend/runner/components/runner_state_paused_badge_spec.js +++ b/spec/frontend/runner/components/runner_paused_badge_spec.js @@ -1,6 +1,6 @@ import { GlBadge } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; -import RunnerStatePausedBadge from '~/runner/components/runner_state_paused_badge.vue'; +import RunnerStatePausedBadge from '~/runner/components/runner_paused_badge.vue'; import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; describe('RunnerTypeBadge', () => { diff --git a/spec/frontend/runner/components/runner_state_locked_badge_spec.js b/spec/frontend/runner/components/runner_state_locked_badge_spec.js deleted file mode 100644 index e92b671f5a1..00000000000 --- a/spec/frontend/runner/components/runner_state_locked_badge_spec.js +++ /dev/null @@ -1,45 +0,0 @@ -import { GlBadge } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; -import RunnerStateLockedBadge from '~/runner/components/runner_state_locked_badge.vue'; -import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; - -describe('RunnerTypeBadge', () => { - let wrapper; - - const findBadge = () => wrapper.findComponent(GlBadge); - const getTooltip = () => getBinding(findBadge().element, 'gl-tooltip'); - - const createComponent = ({ props = {} } = {}) => { - wrapper = shallowMount(RunnerStateLockedBadge, { - propsData: { - ...props, - }, - directives: { - GlTooltip: createMockDirective(), - }, - }); - }; - - beforeEach(() => { - createComponent(); - }); - - afterEach(() => { - wrapper.destroy(); - }); - - it('renders locked state', () => { - expect(wrapper.text()).toBe('locked'); - expect(findBadge().props('variant')).toBe('warning'); - }); - - it('renders tooltip', () => { - expect(getTooltip().value).toBeDefined(); - }); - - it('passes arbitrary attributes to the badge', () => { - createComponent({ props: { size: 'sm' } }); - - expect(findBadge().props('size')).toBe('sm'); - }); -}); diff --git a/spec/frontend/runner/components/runner_type_alert_spec.js b/spec/frontend/runner/components/runner_type_alert_spec.js index e54e499743b..4023c75c9a8 100644 --- a/spec/frontend/runner/components/runner_type_alert_spec.js +++ b/spec/frontend/runner/components/runner_type_alert_spec.js @@ -23,11 +23,11 @@ describe('RunnerTypeAlert', () => { }); describe.each` - type | exampleText | anchor | variant - ${INSTANCE_TYPE} | ${'This runner is available to all groups and projects'} | ${'#shared-runners'} | ${'success'} - ${GROUP_TYPE} | ${'This runner is available to all projects and subgroups in a group'} | ${'#group-runners'} | ${'success'} - ${PROJECT_TYPE} | ${'This runner is associated with one or more projects'} | ${'#specific-runners'} | ${'info'} - `('When it is an $type level runner', ({ type, exampleText, anchor, variant }) => { + type | exampleText | anchor + ${INSTANCE_TYPE} | ${'This runner is available to all groups and projects'} | ${'#shared-runners'} + ${GROUP_TYPE} | ${'This runner is available to all projects and subgroups in a group'} | ${'#group-runners'} + ${PROJECT_TYPE} | ${'This runner is associated with one or more projects'} | ${'#specific-runners'} + `('When it is an $type level runner', ({ type, exampleText, anchor }) => { beforeEach(() => { createComponent({ props: { type } }); }); @@ -36,8 +36,8 @@ describe('RunnerTypeAlert', () => { expect(wrapper.text()).toMatch(exampleText); }); - it(`Shows a ${variant} variant`, () => { - expect(findAlert().props('variant')).toBe(variant); + it(`Shows an "info" variant`, () => { + expect(findAlert().props('variant')).toBe('info'); }); it(`Links to anchor "${anchor}"`, () => { diff --git a/spec/frontend/runner/components/runner_type_badge_spec.js b/spec/frontend/runner/components/runner_type_badge_spec.js index fb344e65389..7bb0a2e6e2f 100644 --- a/spec/frontend/runner/components/runner_type_badge_spec.js +++ b/spec/frontend/runner/components/runner_type_badge_spec.js @@ -26,18 +26,18 @@ describe('RunnerTypeBadge', () => { }); describe.each` - type | text | variant - ${INSTANCE_TYPE} | ${'shared'} | ${'success'} - ${GROUP_TYPE} | ${'group'} | ${'success'} - ${PROJECT_TYPE} | ${'specific'} | ${'info'} - `('displays $type runner', ({ type, text, variant }) => { + type | text + ${INSTANCE_TYPE} | ${'shared'} + ${GROUP_TYPE} | ${'group'} + ${PROJECT_TYPE} | ${'specific'} + `('displays $type runner', ({ type, text }) => { beforeEach(() => { createComponent({ props: { type } }); }); - it(`as "${text}" with a ${variant} variant`, () => { + it(`as "${text}" with an "info" variant`, () => { expect(findBadge().text()).toBe(text); - expect(findBadge().props('variant')).toBe(variant); + expect(findBadge().props('variant')).toBe('info'); }); it('with a tooltip', () => { diff --git a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb index 83a37655ea9..e982f0eb015 100644 --- a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb +++ b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb @@ -18,17 +18,6 @@ RSpec.describe Gitlab::Ci::ArtifactFileReader do expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom') end - context 'when FF ci_new_artifact_file_reader is disabled' do - before do - stub_feature_flags(ci_new_artifact_file_reader: false) - end - - it 'returns the content at the path' do - is_expected.to be_present - expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom') - end - end - context 'when path does not exist' do let(:path) { 'file/does/not/exist.txt' } let(:expected_error) do diff --git a/spec/lib/gitlab/database/gitlab_schema_spec.rb b/spec/lib/gitlab/database/gitlab_schema_spec.rb index 4f94ce2e541..255efc99ff6 100644 --- a/spec/lib/gitlab/database/gitlab_schema_spec.rb +++ b/spec/lib/gitlab/database/gitlab_schema_spec.rb @@ -35,4 +35,24 @@ RSpec.describe Gitlab::Database::GitlabSchema do end end end + + describe '.table_schema' do + using RSpec::Parameterized::TableSyntax + + where(:name, :classification) do + 'ci_builds' | :gitlab_ci + 'my_schema.ci_builds' | :gitlab_ci + 'information_schema.columns' | :gitlab_shared + 'audit_events_part_5fc467ac26' | :gitlab_main + '_test_my_table' | :gitlab_shared + 'pg_attribute' | :gitlab_shared + 'my_other_table' | :undefined_my_other_table + end + + with_them do + subject { described_class.table_schema(name) } + + it { is_expected.to eq(classification) } + end + end end diff --git a/spec/lib/gitlab/database/query_analyzer_spec.rb b/spec/lib/gitlab/database/query_analyzer_spec.rb new file mode 100644 index 00000000000..b8356522e55 --- /dev/null +++ b/spec/lib/gitlab/database/query_analyzer_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::QueryAnalyzer do + let(:analyzer) { double(:query_analyzer) } + + before do + stub_const('Gitlab::Database::QueryAnalyzer::ANALYZERS', [analyzer]) + end + + context 'the hook is enabled by default in specs' do + it 'does process queries and gets normalized SQL' do + expect(analyzer).to receive(:enabled?).and_return(true) + expect(analyzer).to receive(:analyze) do |parsed| + expect(parsed.sql).to include("SELECT $1 FROM projects") + expect(parsed.pg.tables).to eq(%w[projects]) + end + + Project.connection.execute("SELECT 1 FROM projects") + end + end + + describe '#process_sql' do + it 'does not analyze query if not enabled' do + expect(analyzer).to receive(:enabled?).and_return(false) + expect(analyzer).not_to receive(:analyze) + + process_sql("SELECT 1 FROM projects") + end + + it 'does analyze query if enabled' do + expect(analyzer).to receive(:enabled?).and_return(true) + expect(analyzer).to receive(:analyze) do |parsed| + expect(parsed.sql).to eq("SELECT $1 FROM projects") + expect(parsed.pg.tables).to eq(%w[projects]) + end + + process_sql("SELECT 1 FROM projects") + end + + it 'does track exception if query cannot be parsed' do + expect(analyzer).to receive(:enabled?).and_return(true) + expect(analyzer).not_to receive(:analyze) + expect(Gitlab::ErrorTracking).to receive(:track_exception) + + expect { process_sql("invalid query") }.not_to raise_error + end + + it 'does track exception if analyzer raises exception on enabled?' do + expect(analyzer).to receive(:enabled?).and_raise('exception') + expect(analyzer).not_to receive(:analyze) + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) + + expect { process_sql("SELECT 1 FROM projects") }.not_to raise_error + end + + it 'does track exception if analyzer raises exception on analyze' do + expect(analyzer).to receive(:enabled?).and_return(true) + expect(analyzer).to receive(:analyze).and_raise('exception') + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) + + expect { process_sql("SELECT 1 FROM projects") }.not_to raise_error + end + + def process_sql(sql) + ApplicationRecord.connection.load_balancer.read_write do |connection| + described_class.new.send(:process_sql, sql, connection) + end + end + end +end diff --git a/spec/models/acts_as_taggable_on/tag_spec.rb b/spec/models/acts_as_taggable_on/tag_spec.rb new file mode 100644 index 00000000000..4b390bbd0bb --- /dev/null +++ b/spec/models/acts_as_taggable_on/tag_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ActsAsTaggableOn::Tag do + it 'has the same connection as Ci::ApplicationRecord' do + query = 'select current_database()' + + expect(described_class.connection.execute(query).first).to eq(Ci::ApplicationRecord.connection.execute(query).first) + expect(described_class.retrieve_connection.execute(query).first).to eq(Ci::ApplicationRecord.retrieve_connection.execute(query).first) + end + + it 'has the same sticking as Ci::ApplicationRecord' do + expect(described_class.sticking).to eq(Ci::ApplicationRecord.sticking) + end +end diff --git a/spec/models/acts_as_taggable_on/tagging_spec.rb b/spec/models/acts_as_taggable_on/tagging_spec.rb new file mode 100644 index 00000000000..4520a0aaf70 --- /dev/null +++ b/spec/models/acts_as_taggable_on/tagging_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ActsAsTaggableOn::Tagging do + it 'has the same connection as Ci::ApplicationRecord' do + query = 'select current_database()' + + expect(described_class.connection.execute(query).first).to eq(Ci::ApplicationRecord.connection.execute(query).first) + expect(described_class.retrieve_connection.execute(query).first).to eq(Ci::ApplicationRecord.retrieve_connection.execute(query).first) + end + + it 'has the same sticking as Ci::ApplicationRecord' do + expect(described_class.sticking).to eq(Ci::ApplicationRecord.sticking) + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index ce0442f27a3..c326b2d6fcd 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2648,14 +2648,6 @@ RSpec.describe Group do end it_behaves_like 'returns namespaces with disabled email' - - context 'when feature flag :linear_group_ancestor_scopes is disabled' do - before do - stub_feature_flags(linear_group_ancestor_scopes: false) - end - - it_behaves_like 'returns namespaces with disabled email' - end end describe '.timelogs' do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 02cfb594348..46a22f64591 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -284,7 +284,7 @@ RSpec.describe Namespace do end end - context 'creating a default Namespace' do + context 'creating a Namespace with nil type' do let(:namespace_type) { nil } it 'is the correct type of namespace' do @@ -295,7 +295,7 @@ RSpec.describe Namespace do end context 'creating an unknown Namespace type' do - let(:namespace_type) { 'One' } + let(:namespace_type) { 'nonsense' } it 'creates a default Namespace' do expect(Namespace.find(namespace.id)).to be_a(Namespace) diff --git a/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb b/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb new file mode 100644 index 00000000000..3da702c55d7 --- /dev/null +++ b/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Setting issues crm contacts' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:contacts) { create_list(:contact, 4, group: group) } + + let(:issue) { create(:issue, project: project) } + let(:operation_mode) { Types::MutationOperationModeEnum.default_mode } + let(:crm_contact_ids) { [global_id_of(contacts[1]), global_id_of(contacts[2])] } + let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" } + + let(:mutation) do + variables = { + project_path: issue.project.full_path, + iid: issue.iid.to_s, + operation_mode: operation_mode, + crm_contact_ids: crm_contact_ids + } + + graphql_mutation(:issue_set_crm_contacts, variables, + <<-QL.strip_heredoc + clientMutationId + errors + issue { + customerRelationsContacts { + nodes { + id + } + } + } + QL + ) + end + + def mutation_response + graphql_mutation_response(:issue_set_crm_contacts) + end + + before do + create(:issue_customer_relations_contact, issue: issue, contact: contacts[0]) + create(:issue_customer_relations_contact, issue: issue, contact: contacts[1]) + end + + context 'when the user has no permission' do + it 'returns expected error' do + error = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).to include(a_hash_including('message' => error)) + end + end + + context 'when the user has permission' do + before do + group.add_reporter(user) + end + + context 'when the feature is disabled' do + before do + stub_feature_flags(customer_relations: false) + end + + it 'raises expected error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).to include(a_hash_including('message' => 'Feature disabled')) + end + end + + context 'replace' do + it 'updates the issue with correct contacts' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id)) + .to match_array([global_id_of(contacts[1]), global_id_of(contacts[2])]) + end + end + + context 'append' do + let(:crm_contact_ids) { [global_id_of(contacts[3])] } + let(:operation_mode) { Types::MutationOperationModeEnum.enum[:append] } + + it 'updates the issue with correct contacts' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id)) + .to match_array([global_id_of(contacts[0]), global_id_of(contacts[1]), global_id_of(contacts[3])]) + end + end + + context 'remove' do + let(:crm_contact_ids) { [global_id_of(contacts[0])] } + let(:operation_mode) { Types::MutationOperationModeEnum.enum[:remove] } + + it 'updates the issue with correct contacts' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id)) + .to match_array([global_id_of(contacts[1])]) + end + end + + context 'when the contact does not exist' do + let(:crm_contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] } + + it 'returns expected error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_data_at(:issue_set_crm_contacts, :errors)) + .to match_array(["Issue customer relations contacts #{non_existing_record_id}: #{does_not_exist_or_no_permission}"]) + end + end + + context 'when the contact belongs to a different group' do + let(:group2) { create(:group) } + let(:contact) { create(:contact, group: group2) } + let(:crm_contact_ids) { [global_id_of(contact)] } + + before do + group2.add_reporter(user) + end + + it 'returns expected error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_data_at(:issue_set_crm_contacts, :errors)) + .to match_array(["Issue customer relations contacts #{contact.id}: #{does_not_exist_or_no_permission}"]) + end + end + + context 'when attempting to add more than 6' do + let(:operation_mode) { Types::MutationOperationModeEnum.enum[:append] } + let(:gid) { global_id_of(contacts[0]) } + let(:crm_contact_ids) { [gid, gid, gid, gid, gid, gid, gid] } + + it 'returns expected error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_data_at(:issue_set_crm_contacts, :errors)) + .to match_array(["You can only add up to 6 contacts at one time"]) + end + end + + context 'when trying to remove non-existent contact' do + let(:operation_mode) { Types::MutationOperationModeEnum.enum[:remove] } + let(:crm_contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] } + + it 'raises expected error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_data_at(:issue_set_crm_contacts, :errors)).to be_empty + end + end + end +end diff --git a/spec/requests/api/v3/github_spec.rb b/spec/requests/api/v3/github_spec.rb index 255f53e4c7c..6d8ae226ce4 100644 --- a/spec/requests/api/v3/github_spec.rb +++ b/spec/requests/api/v3/github_spec.rb @@ -6,7 +6,7 @@ RSpec.describe API::V3::Github do let_it_be(:user) { create(:user) } let_it_be(:unauthorized_user) { create(:user) } let_it_be(:admin) { create(:user, :admin) } - let_it_be(:project) { create(:project, :repository, creator: user) } + let_it_be_with_reload(:project) { create(:project, :repository, creator: user) } before do project.add_maintainer(user) @@ -506,11 +506,18 @@ RSpec.describe API::V3::Github do describe 'GET /repos/:namespace/:project/commits/:sha' do let(:commit) { project.repository.commit } - let(:commit_id) { commit.id } + + def call_api(commit_id: commit.id) + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/commits/#{commit_id}", user) + end + + def response_diff_files(response) + Gitlab::Json.parse(response.body)['files'] + end context 'authenticated' do - it 'returns commit with github format' do - jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/commits/#{commit_id}", user) + it 'returns commit with github format', :aggregate_failures do + call_api expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('entities/github/commit') @@ -519,36 +526,130 @@ RSpec.describe API::V3::Github do it 'returns 200 when project path include a dot' do project.update!(path: 'foo.bar') - jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/commits/#{commit_id}", user) + call_api expect(response).to have_gitlab_http_status(:ok) end - it 'returns 200 when namespace path include a dot' do - group = create(:group, path: 'foo.bar') - project = create(:project, :repository, group: group) - project.add_reporter(user) + context 'when namespace path includes a dot' do + let(:group) { create(:group, path: 'foo.bar') } + let(:project) { create(:project, :repository, group: group) } - jira_get v3_api("/repos/#{group.path}/#{project.path}/commits/#{commit_id}", user) + it 'returns 200 when namespace path include a dot' do + project.add_reporter(user) - expect(response).to have_gitlab_http_status(:ok) + call_api + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when the Gitaly `CommitDiff` RPC times out', :use_clean_rails_memory_store_caching do + let(:commit_diff_args) { [project.repository_storage, :diff_service, :commit_diff, any_args] } + + before do + allow(Gitlab::GitalyClient).to receive(:call) + .and_call_original + end + + it 'handles the error, logs it, and returns empty diff files', :aggregate_failures do + allow(Gitlab::GitalyClient).to receive(:call) + .with(*commit_diff_args) + .and_raise(GRPC::DeadlineExceeded) + + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .with an_instance_of(GRPC::DeadlineExceeded) + + call_api + + expect(response).to have_gitlab_http_status(:ok) + expect(response_diff_files(response)).to be_blank + end + + it 'does not handle the error when feature flag is disabled', :aggregate_failures do + stub_feature_flags(api_v3_commits_skip_diff_files: false) + + allow(Gitlab::GitalyClient).to receive(:call) + .with(*commit_diff_args) + .and_raise(GRPC::DeadlineExceeded) + + call_api + + expect(response).to have_gitlab_http_status(:error) + end + + it 'only calls Gitaly once for all attempts within a period of time', :aggregate_failures do + expect(Gitlab::GitalyClient).to receive(:call) + .with(*commit_diff_args) + .once # <- once + .and_raise(GRPC::DeadlineExceeded) + + 3.times do + call_api + + expect(response).to have_gitlab_http_status(:ok) + expect(response_diff_files(response)).to be_blank + end + end + + it 'calls Gitaly again after a period of time', :aggregate_failures do + expect(Gitlab::GitalyClient).to receive(:call) + .with(*commit_diff_args) + .twice # <- twice + .and_raise(GRPC::DeadlineExceeded) + + call_api + + expect(response).to have_gitlab_http_status(:ok) + expect(response_diff_files(response)).to be_blank + + travel_to((described_class::GITALY_TIMEOUT_CACHE_EXPIRY + 1.second).from_now) do + call_api + + expect(response).to have_gitlab_http_status(:ok) + expect(response_diff_files(response)).to be_blank + end + end + + it 'uses a unique cache key, allowing other calls to succeed' do + cache_key = [described_class::GITALY_TIMEOUT_CACHE_KEY, project.id, commit.cache_key].join(':') + Rails.cache.write(cache_key, 1) + + expect(Gitlab::GitalyClient).to receive(:call) + .with(*commit_diff_args) + .once # <- once + + call_api + + expect(response).to have_gitlab_http_status(:ok) + expect(response_diff_files(response)).to be_blank + + call_api(commit_id: commit.parent.id) + + expect(response).to have_gitlab_http_status(:ok) + expect(response_diff_files(response).length).to eq(1) + end end end context 'unauthenticated' do + let(:user) { nil } + it 'returns 401' do - jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/commits/#{commit_id}", nil) + call_api expect(response).to have_gitlab_http_status(:unauthorized) end end context 'unauthorized' do + let(:user) { unauthorized_user } + it 'returns 404 when lower access level' do - project.add_guest(unauthorized_user) + project.add_guest(user) - jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/commits/#{commit_id}", - unauthorized_user) + call_api expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/services/issues/set_crm_contacts_service_spec.rb b/spec/services/issues/set_crm_contacts_service_spec.rb new file mode 100644 index 00000000000..65b22fe3b35 --- /dev/null +++ b/spec/services/issues/set_crm_contacts_service_spec.rb @@ -0,0 +1,162 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Issues::SetCrmContactsService do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:contacts) { create_list(:contact, 4, group: group) } + + let(:issue) { create(:issue, project: project) } + let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" } + + before do + create(:issue_customer_relations_contact, issue: issue, contact: contacts[0]) + create(:issue_customer_relations_contact, issue: issue, contact: contacts[1]) + end + + subject(:set_crm_contacts) do + described_class.new(project: project, current_user: user, params: params).execute(issue) + end + + describe '#execute' do + context 'when the user has no permission' do + let(:params) { { crm_contact_ids: [contacts[1].id, contacts[2].id] } } + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array(['You have insufficient permissions to set customer relations contacts for this issue']) + end + end + + context 'when user has permission' do + before do + group.add_reporter(user) + end + + context 'when the contact does not exist' do + let(:params) { { crm_contact_ids: [non_existing_record_id] } } + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array(["Issue customer relations contacts #{non_existing_record_id}: #{does_not_exist_or_no_permission}"]) + end + end + + context 'when the contact belongs to a different group' do + let(:group2) { create(:group) } + let(:contact) { create(:contact, group: group2) } + let(:params) { { crm_contact_ids: [contact.id] } } + + before do + group2.add_reporter(user) + end + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array(["Issue customer relations contacts #{contact.id}: #{does_not_exist_or_no_permission}"]) + end + end + + context 'replace' do + let(:params) { { crm_contact_ids: [contacts[1].id, contacts[2].id] } } + + it 'updates the issue with correct contacts' do + response = set_crm_contacts + + expect(response).to be_success + expect(issue.customer_relations_contacts).to match_array([contacts[1], contacts[2]]) + end + end + + context 'add' do + let(:params) { { add_crm_contact_ids: [contacts[3].id] } } + + it 'updates the issue with correct contacts' do + response = set_crm_contacts + + expect(response).to be_success + expect(issue.customer_relations_contacts).to match_array([contacts[0], contacts[1], contacts[3]]) + end + end + + context 'remove' do + let(:params) { { remove_crm_contact_ids: [contacts[0].id] } } + + it 'updates the issue with correct contacts' do + response = set_crm_contacts + + expect(response).to be_success + expect(issue.customer_relations_contacts).to match_array([contacts[1]]) + end + end + + context 'when attempting to add more than 6' do + let(:id) { contacts[0].id } + let(:params) { { add_crm_contact_ids: [id, id, id, id, id, id, id] } } + + it 'returns expected error message' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array(['You can only add up to 6 contacts at one time']) + end + end + + context 'when trying to remove non-existent contact' do + let(:params) { { remove_crm_contact_ids: [non_existing_record_id] } } + + it 'returns expected error message' do + response = set_crm_contacts + + expect(response).to be_success + expect(response.message).to be_nil + end + end + + context 'when combining params' do + let(:error_invalid_params) { 'You cannot combine crm_contact_ids with add_crm_contact_ids or remove_crm_contact_ids' } + + context 'add and remove' do + let(:params) { { remove_crm_contact_ids: [contacts[1].id], add_crm_contact_ids: [contacts[3].id] } } + + it 'updates the issue with correct contacts' do + response = set_crm_contacts + + expect(response).to be_success + expect(issue.customer_relations_contacts).to match_array([contacts[0], contacts[3]]) + end + end + + context 'replace and remove' do + let(:params) { { crm_contact_ids: [contacts[3].id], remove_crm_contact_ids: [contacts[0].id] } } + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array([error_invalid_params]) + end + end + + context 'replace and add' do + let(:params) { { crm_contact_ids: [contacts[3].id], add_crm_contact_ids: [contacts[1].id] } } + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array([error_invalid_params]) + end + end + end + end + end +end diff --git a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb index 289bbf4540e..a41ba8216cc 100644 --- a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb @@ -41,322 +41,320 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_ describe '#execute' do subject { service.execute } - shared_examples 'reading and removing tags' do |caching_enabled: true| - context 'when no params are specified' do - let(:params) { {} } + context 'when no params are specified' do + let(:params) { {} } - it 'does not remove anything' do - expect_any_instance_of(Projects::ContainerRepository::DeleteTagsService) - .not_to receive(:execute) - expect_no_caching + it 'does not remove anything' do + expect_any_instance_of(Projects::ContainerRepository::DeleteTagsService) + .not_to receive(:execute) + expect_no_caching - is_expected.to eq(expected_service_response(before_truncate_size: 0, after_truncate_size: 0, before_delete_size: 0)) - end + is_expected.to eq(expected_service_response(before_truncate_size: 0, after_truncate_size: 0, before_delete_size: 0)) end + end - context 'when regex matching everything is specified' do - shared_examples 'removes all matches' do - it 'does remove all tags except latest' do - expect_no_caching + context 'when regex matching everything is specified' do + shared_examples 'removes all matches' do + it 'does remove all tags except latest' do + expect_no_caching - expect_delete(%w(A Ba Bb C D E)) + expect_delete(%w(A Ba Bb C D E)) - is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C D E))) - end + is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C D E))) end + end + + let(:params) do + { 'name_regex_delete' => '.*' } + end + it_behaves_like 'removes all matches' + + context 'with deprecated name_regex param' do let(:params) do - { 'name_regex_delete' => '.*' } + { 'name_regex' => '.*' } end it_behaves_like 'removes all matches' + end + end - context 'with deprecated name_regex param' do - let(:params) do - { 'name_regex' => '.*' } - end + context 'with invalid regular expressions' do + shared_examples 'handling an invalid regex' do + it 'keeps all tags' do + expect_no_caching - it_behaves_like 'removes all matches' + expect(Projects::ContainerRepository::DeleteTagsService) + .not_to receive(:new) + + subject end - end - context 'with invalid regular expressions' do - shared_examples 'handling an invalid regex' do - it 'keeps all tags' do - expect_no_caching + it { is_expected.to eq(status: :error, message: 'invalid regex') } - expect(Projects::ContainerRepository::DeleteTagsService) - .not_to receive(:new) + it 'calls error tracking service' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original - subject - end + subject + end + end - it { is_expected.to eq(status: :error, message: 'invalid regex') } + context 'when name_regex_delete is invalid' do + let(:params) { { 'name_regex_delete' => '*test*' } } - it 'calls error tracking service' do - expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original + it_behaves_like 'handling an invalid regex' + end - subject - end - end + context 'when name_regex is invalid' do + let(:params) { { 'name_regex' => '*test*' } } - context 'when name_regex_delete is invalid' do - let(:params) { { 'name_regex_delete' => '*test*' } } + it_behaves_like 'handling an invalid regex' + end - it_behaves_like 'handling an invalid regex' - end + context 'when name_regex_keep is invalid' do + let(:params) { { 'name_regex_keep' => '*test*' } } - context 'when name_regex is invalid' do - let(:params) { { 'name_regex' => '*test*' } } + it_behaves_like 'handling an invalid regex' + end + end - it_behaves_like 'handling an invalid regex' - end + context 'when delete regex matching specific tags is used' do + let(:params) do + { 'name_regex_delete' => 'C|D' } + end - context 'when name_regex_keep is invalid' do - let(:params) { { 'name_regex_keep' => '*test*' } } + it 'does remove C and D' do + expect_delete(%w(C D)) - it_behaves_like 'handling an invalid regex' - end + expect_no_caching + + is_expected.to eq(expected_service_response(deleted: %w(C D), before_truncate_size: 2, after_truncate_size: 2, before_delete_size: 2)) end - context 'when delete regex matching specific tags is used' do + context 'with overriding allow regex' do let(:params) do - { 'name_regex_delete' => 'C|D' } + { 'name_regex_delete' => 'C|D', + 'name_regex_keep' => 'C' } end - it 'does remove C and D' do - expect_delete(%w(C D)) + it 'does not remove C' do + expect_delete(%w(D)) expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(C D), before_truncate_size: 2, after_truncate_size: 2, before_delete_size: 2)) + is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1)) end + end - context 'with overriding allow regex' do - let(:params) do - { 'name_regex_delete' => 'C|D', - 'name_regex_keep' => 'C' } - end + context 'with name_regex_delete overriding deprecated name_regex' do + let(:params) do + { 'name_regex' => 'C|D', + 'name_regex_delete' => 'D' } + end - it 'does not remove C' do - expect_delete(%w(D)) + it 'does not remove C' do + expect_delete(%w(D)) - expect_no_caching + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1)) - end + is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1)) end + end + end - context 'with name_regex_delete overriding deprecated name_regex' do - let(:params) do - { 'name_regex' => 'C|D', - 'name_regex_delete' => 'D' } - end + context 'with allow regex value' do + let(:params) do + { 'name_regex_delete' => '.*', + 'name_regex_keep' => 'B.*' } + end - it 'does not remove C' do - expect_delete(%w(D)) + it 'does not remove B*' do + expect_delete(%w(A C D E)) - expect_no_caching + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1)) - end - end + is_expected.to eq(expected_service_response(deleted: %w(A C D E), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4)) + end + end + + context 'when keeping only N tags' do + let(:params) do + { 'name_regex' => 'A|B.*|C', + 'keep_n' => 1 } end - context 'with allow regex value' do - let(:params) do - { 'name_regex_delete' => '.*', - 'name_regex_keep' => 'B.*' } - end + it 'sorts tags by date' do + expect_delete(%w(Bb Ba C)) - it 'does not remove B*' do - expect_delete(%w(A C D E)) + expect_no_caching - expect_no_caching + expect(service).to receive(:order_by_date).and_call_original - is_expected.to eq(expected_service_response(deleted: %w(A C D E), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4)) - end + is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 3)) end + end - context 'when keeping only N tags' do - let(:params) do - { 'name_regex' => 'A|B.*|C', - 'keep_n' => 1 } - end + context 'when not keeping N tags' do + let(:params) do + { 'name_regex' => 'A|B.*|C' } + end - it 'sorts tags by date' do - expect_delete(%w(Bb Ba C)) + it 'does not sort tags by date' do + expect_delete(%w(A Ba Bb C)) - expect_no_caching + expect_no_caching - expect(service).to receive(:order_by_date).and_call_original + expect(service).not_to receive(:order_by_date) - is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 3)) - end + is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4)) end + end - context 'when not keeping N tags' do - let(:params) do - { 'name_regex' => 'A|B.*|C' } - end - - it 'does not sort tags by date' do - expect_delete(%w(A Ba Bb C)) + context 'when removing keeping only 3' do + let(:params) do + { 'name_regex_delete' => '.*', + 'keep_n' => 3 } + end - expect_no_caching + it 'does remove B* and C as they are the oldest' do + expect_delete(%w(Bb Ba C)) - expect(service).not_to receive(:order_by_date) + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4)) - end + is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) end + end - context 'when removing keeping only 3' do - let(:params) do - { 'name_regex_delete' => '.*', - 'keep_n' => 3 } - end + context 'when removing older than 1 day' do + let(:params) do + { 'name_regex_delete' => '.*', + 'older_than' => '1 day' } + end - it 'does remove B* and C as they are the oldest' do - expect_delete(%w(Bb Ba C)) + it 'does remove B* and C as they are older than 1 day' do + expect_delete(%w(Ba Bb C)) - expect_no_caching + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) - end + is_expected.to eq(expected_service_response(deleted: %w(Ba Bb C), before_delete_size: 3)) end + end - context 'when removing older than 1 day' do - let(:params) do - { 'name_regex_delete' => '.*', - 'older_than' => '1 day' } - end + context 'when combining all parameters' do + let(:params) do + { 'name_regex_delete' => '.*', + 'keep_n' => 1, + 'older_than' => '1 day' } + end - it 'does remove B* and C as they are older than 1 day' do - expect_delete(%w(Ba Bb C)) + it 'does remove B* and C' do + expect_delete(%w(Bb Ba C)) - expect_no_caching + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(Ba Bb C), before_delete_size: 3)) - end + is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) end + end + + context 'when running a container_expiration_policy' do + let(:user) { nil } - context 'when combining all parameters' do + context 'with valid container_expiration_policy param' do let(:params) do { 'name_regex_delete' => '.*', 'keep_n' => 1, - 'older_than' => '1 day' } + 'older_than' => '1 day', + 'container_expiration_policy' => true } end - it 'does remove B* and C' do - expect_delete(%w(Bb Ba C)) + it 'succeeds without a user' do + expect_delete(%w(Bb Ba C), container_expiration_policy: true) - expect_no_caching + expect_caching is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) end end - context 'when running a container_expiration_policy' do - let(:user) { nil } - - context 'with valid container_expiration_policy param' do - let(:params) do - { 'name_regex_delete' => '.*', - 'keep_n' => 1, - 'older_than' => '1 day', - 'container_expiration_policy' => true } - end - - it 'succeeds without a user' do - expect_delete(%w(Bb Ba C), container_expiration_policy: true) - - caching_enabled ? expect_caching : expect_no_caching - - is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) - end + context 'without container_expiration_policy param' do + let(:params) do + { 'name_regex_delete' => '.*', + 'keep_n' => 1, + 'older_than' => '1 day' } end - context 'without container_expiration_policy param' do - let(:params) do - { 'name_regex_delete' => '.*', - 'keep_n' => 1, - 'older_than' => '1 day' } - end - - it 'fails' do - is_expected.to eq(status: :error, message: 'access denied') - end + it 'fails' do + is_expected.to eq(status: :error, message: 'access denied') end end + end - context 'truncating the tags list' do - let(:params) do - { - 'name_regex_delete' => '.*', - 'keep_n' => 1 - } - end + context 'truncating the tags list' do + let(:params) do + { + 'name_regex_delete' => '.*', + 'keep_n' => 1 + } + end - shared_examples 'returning the response' do |status:, original_size:, before_truncate_size:, after_truncate_size:, before_delete_size:| - it 'returns the response' do - expect_no_caching + shared_examples 'returning the response' do |status:, original_size:, before_truncate_size:, after_truncate_size:, before_delete_size:| + it 'returns the response' do + expect_no_caching - result = subject + result = subject - service_response = expected_service_response( - status: status, - original_size: original_size, - before_truncate_size: before_truncate_size, - after_truncate_size: after_truncate_size, - before_delete_size: before_delete_size, - deleted: nil - ) + service_response = expected_service_response( + status: status, + original_size: original_size, + before_truncate_size: before_truncate_size, + after_truncate_size: after_truncate_size, + before_delete_size: before_delete_size, + deleted: nil + ) - expect(result).to eq(service_response) - end + expect(result).to eq(service_response) end + end - where(:feature_flag_enabled, :max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do - false | 10 | :success | :success | false - false | 10 | :error | :error | false - false | 3 | :success | :success | false - false | 3 | :error | :error | false - false | 0 | :success | :success | false - false | 0 | :error | :error | false - true | 10 | :success | :success | false - true | 10 | :error | :error | false - true | 3 | :success | :error | true - true | 3 | :error | :error | true - true | 0 | :success | :success | false - true | 0 | :error | :error | false - end + where(:feature_flag_enabled, :max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do + false | 10 | :success | :success | false + false | 10 | :error | :error | false + false | 3 | :success | :success | false + false | 3 | :error | :error | false + false | 0 | :success | :success | false + false | 0 | :error | :error | false + true | 10 | :success | :success | false + true | 10 | :error | :error | false + true | 3 | :success | :error | true + true | 3 | :error | :error | true + true | 0 | :success | :success | false + true | 0 | :error | :error | false + end - with_them do - before do - stub_feature_flags(container_registry_expiration_policies_throttling: feature_flag_enabled) - stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size) - allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service| - expect(service).to receive(:execute).and_return(status: delete_tags_service_status) - end + with_them do + before do + stub_feature_flags(container_registry_expiration_policies_throttling: feature_flag_enabled) + stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size) + allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service| + expect(service).to receive(:execute).and_return(status: delete_tags_service_status) end + end - original_size = 7 - keep_n = 1 + original_size = 7 + keep_n = 1 - it_behaves_like( - 'returning the response', - status: params[:expected_status], - original_size: original_size, - before_truncate_size: original_size - keep_n, - after_truncate_size: params[:expected_truncated] ? params[:max_list_size] + keep_n : original_size - keep_n, - before_delete_size: params[:expected_truncated] ? params[:max_list_size] : original_size - keep_n - 1 # one tag is filtered out with older_than filter - ) - end + it_behaves_like( + 'returning the response', + status: params[:expected_status], + original_size: original_size, + before_truncate_size: original_size - keep_n, + after_truncate_size: params[:expected_truncated] ? params[:max_list_size] + keep_n : original_size - keep_n, + before_delete_size: params[:expected_truncated] ? params[:max_list_size] : original_size - keep_n - 1 # one tag is filtered out with older_than filter + ) end end - context 'caching' do + context 'caching', :freeze_time do let(:params) do { 'name_regex_delete' => '.*', @@ -381,17 +379,12 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_ before do expect_delete(%w(Bb Ba C), container_expiration_policy: true) - travel_to(Time.zone.local(2021, 9, 2, 12, 0, 0)) # We froze time so we need to set the created_at stubs again stub_digest_config('sha256:configA', 1.hour.ago) stub_digest_config('sha256:configB', 5.days.ago) stub_digest_config('sha256:configC', 1.month.ago) end - after do - travel_back - end - it 'caches the created_at values' do ::Gitlab::Redis::Cache.with do |redis| expect_mget(redis, tags_and_created_ats.keys) @@ -450,32 +443,6 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_ DateTime.rfc3339(date_time.rfc3339).rfc3339 end end - - context 'with container_registry_expiration_policies_caching enabled for the project' do - before do - stub_feature_flags(container_registry_expiration_policies_caching: project) - end - - it_behaves_like 'reading and removing tags', caching_enabled: true - end - - context 'with container_registry_expiration_policies_caching disabled' do - before do - stub_feature_flags(container_registry_expiration_policies_caching: false) - end - - it_behaves_like 'reading and removing tags', caching_enabled: false - end - - context 'with container_registry_expiration_policies_caching not enabled for the project' do - let_it_be(:another_project) { create(:project) } - - before do - stub_feature_flags(container_registry_expiration_policies_caching: another_project) - end - - it_behaves_like 'reading and removing tags', caching_enabled: false - end end private diff --git a/tooling/bin/find_change_diffs b/tooling/bin/find_change_diffs index 8ac26d56476..7857945ea74 100755 --- a/tooling/bin/find_change_diffs +++ b/tooling/bin/find_change_diffs @@ -5,11 +5,22 @@ require 'gitlab' require 'pathname' # This script saves the diffs of changes in an MR to the directory specified as the first argument +# +# It exits with a success code if diffs are found and saved, or if there are no changes, including if the script runs in +# a pipeline that is not for a merge request. gitlab_token = ENV.fetch('PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE') gitlab_endpoint = ENV.fetch('CI_API_V4_URL') -mr_project_path = ENV.fetch('CI_MERGE_REQUEST_PROJECT_PATH') -mr_iid = ENV.fetch('CI_MERGE_REQUEST_IID') +mr_project_path = ENV['CI_MERGE_REQUEST_PROJECT_PATH'] +mr_iid = ENV['CI_MERGE_REQUEST_IID'] + +puts "CI_MERGE_REQUEST_PROJECT_PATH is missing." if mr_project_path.to_s.empty? +puts "CI_MERGE_REQUEST_IID is missing." if mr_iid.to_s.empty? + +unless mr_project_path && mr_iid + puts "Exiting as this does not appear to be a merge request pipeline." + exit +end abort("ERROR: Please specify a directory to write MR diffs into.") if ARGV.empty? output_diffs_dir = Pathname.new(ARGV.shift).expand_path diff --git a/tooling/bin/qa/check_if_only_quarantined_specs b/tooling/bin/qa/check_if_only_quarantined_specs deleted file mode 100755 index 8a36761c58a..00000000000 --- a/tooling/bin/qa/check_if_only_quarantined_specs +++ /dev/null @@ -1,18 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -require 'pathname' - -# This script assumes the first argument is a directory of files containing diffs of changes from an MR. It exits with a -# success code if all diffs add a line that quarantines a test. If any diffs are not specs, or they are specs that don't -# quarantine a test, it exits with code 1 to indicate failure (i.e., there was _not_ only quarantined specs). - -abort("ERROR: Please specify the directory containing MR diffs.") if ARGV.empty? -diffs_dir = Pathname.new(ARGV.shift).expand_path - -diffs_dir.glob('**/*').each do |path| - next if path.directory? - - exit 1 unless path.to_s.end_with?('_spec.rb.diff') - exit 1 unless path.read.match?(/^\+.*, quarantine:/) -end diff --git a/tooling/bin/qa/package_and_qa_check b/tooling/bin/qa/package_and_qa_check new file mode 100755 index 00000000000..21deb0fcd2d --- /dev/null +++ b/tooling/bin/qa/package_and_qa_check @@ -0,0 +1,45 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require 'pathname' + +# This script checks if the package-and-qa job should trigger downstream pipelines to run the QA suite. +# +# It assumes the first argument is a directory of files containing diffs of changes from an MR +# (e.g., created by tooling/bin/find_change_diffs). It exits with a success code if there are no diffs, or if the diffs +# are suitable to run QA tests. +# +# The script will abort (exit code 1) if the argument is missing. +# +# The following condition will result in a failure code (2), indicating that package-and-qa should not run: +# +# - If the changes only include tests being put in quarantine + +abort("ERROR: Please specify the directory containing MR diffs.") if ARGV.empty? +diffs_dir = Pathname.new(ARGV.shift).expand_path + +# Run package-and-qa if there are no diffs. E.g., in scheduled pipelines +exit 0 if diffs_dir.glob('**/*').empty? + +files_count = 0 +specs_count = 0 +quarantine_specs_count = 0 + +diffs_dir.glob('**/*').each do |path| + next if path.directory? + + files_count += 1 + next unless path.to_s.end_with?('_spec.rb.diff') + + specs_count += 1 + quarantine_specs_count += 1 if path.read.match?(/^\+.*, quarantine:/) +end + +# Run package-and-qa if there are no specs. E.g., when the MR changes QA framework files. +exit 0 if specs_count == 0 + +# Skip package-and-qa if there are only specs being put in quarantine. +exit 2 if quarantine_specs_count == specs_count && quarantine_specs_count == files_count + +# Run package-and-qa under any other circumstances. E.g., if there are specs being put in quarantine but there are also +# other changes that might need to be tested. |