diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-11 06:10:58 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-11 06:10:58 +0000 |
| commit | f31ef3fd5548f9ffd5740b6aaca8672c27e34b42 (patch) | |
| tree | 9f270beb0c4cad85b2a50eb25eb6712966cd1ddf | |
| parent | f60515eae2fc00c56742462826ae00826eb8826e (diff) | |
| download | gitlab-ce-f31ef3fd5548f9ffd5740b6aaca8672c27e34b42.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
84 files changed, 1292 insertions, 364 deletions
diff --git a/app/assets/javascripts/issuable/bulk_update_sidebar/issuable_bulk_update_sidebar.js b/app/assets/javascripts/issuable/bulk_update_sidebar/issuable_bulk_update_sidebar.js index 42ac7d4ef9f..d46354e240a 100644 --- a/app/assets/javascripts/issuable/bulk_update_sidebar/issuable_bulk_update_sidebar.js +++ b/app/assets/javascripts/issuable/bulk_update_sidebar/issuable_bulk_update_sidebar.js @@ -1,7 +1,7 @@ /* eslint-disable class-methods-use-this, no-new */ import $ from 'jquery'; -import issuableEventHub from '~/issues_list/eventhub'; +import issuableEventHub from '~/issues/list/eventhub'; import LabelsSelect from '~/labels/labels_select'; import MilestoneSelect from '~/milestones/milestone_select'; import IssuableBulkUpdateActions from './issuable_bulk_update_actions'; diff --git a/app/assets/javascripts/issues_list/components/issue_card_time_info.vue b/app/assets/javascripts/issues/list/components/issue_card_time_info.vue index aece7372182..aece7372182 100644 --- a/app/assets/javascripts/issues_list/components/issue_card_time_info.vue +++ b/app/assets/javascripts/issues/list/components/issue_card_time_info.vue diff --git a/app/assets/javascripts/issues_list/components/issues_list_app.vue b/app/assets/javascripts/issues/list/components/issues_list_app.vue index b1a97a39869..8b15e801f02 100644 --- a/app/assets/javascripts/issues_list/components/issues_list_app.vue +++ b/app/assets/javascripts/issues/list/components/issues_list_app.vue @@ -11,9 +11,9 @@ import { import * as Sentry from '@sentry/browser'; import fuzzaldrinPlus from 'fuzzaldrin-plus'; import { orderBy } from 'lodash'; -import getIssuesQuery from 'ee_else_ce/issues_list/queries/get_issues.query.graphql'; -import getIssuesCountsQuery from 'ee_else_ce/issues_list/queries/get_issues_counts.query.graphql'; -import IssueCardTimeInfo from 'ee_else_ce/issues_list/components/issue_card_time_info.vue'; +import getIssuesQuery from 'ee_else_ce/issues/list/queries/get_issues.query.graphql'; +import getIssuesCountsQuery from 'ee_else_ce/issues/list/queries/get_issues_counts.query.graphql'; +import IssueCardTimeInfo from 'ee_else_ce/issues/list/components/issue_card_time_info.vue'; import createFlash, { FLASH_TYPES } from '~/flash'; import { TYPE_USER } from '~/graphql_shared/constants'; import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; @@ -41,7 +41,7 @@ import { TOKEN_TYPE_TYPE, UPDATED_DESC, urlSortParams, -} from '~/issues_list/constants'; +} from '~/issues/list/constants'; import { convertToApiParams, convertToSearchQuery, @@ -51,7 +51,7 @@ import { getInitialPageParams, getSortKey, getSortOptions, -} from '~/issues_list/utils'; +} from '~/issues/list/utils'; import axios from '~/lib/utils/axios_utils'; import { scrollUp } from '~/lib/utils/scroll_utils'; import { getParameterByName, joinPaths } from '~/lib/utils/url_utility'; diff --git a/app/assets/javascripts/issues_list/components/jira_issues_import_status_app.vue b/app/assets/javascripts/issues/list/components/jira_issues_import_status_app.vue index fb1dbef666c..fb1dbef666c 100644 --- a/app/assets/javascripts/issues_list/components/jira_issues_import_status_app.vue +++ b/app/assets/javascripts/issues/list/components/jira_issues_import_status_app.vue diff --git a/app/assets/javascripts/issues_list/components/new_issue_dropdown.vue b/app/assets/javascripts/issues/list/components/new_issue_dropdown.vue index e749579af80..71f84050ba8 100644 --- a/app/assets/javascripts/issues_list/components/new_issue_dropdown.vue +++ b/app/assets/javascripts/issues/list/components/new_issue_dropdown.vue @@ -7,7 +7,7 @@ import { GlSearchBoxByType, } from '@gitlab/ui'; import createFlash from '~/flash'; -import searchProjectsQuery from '~/issues_list/queries/search_projects.query.graphql'; +import searchProjectsQuery from '~/issues/list/queries/search_projects.query.graphql'; import { DASH_SCOPE, joinPaths } from '~/lib/utils/url_utility'; import { __, sprintf } from '~/locale'; import { DEBOUNCE_DELAY } from '~/vue_shared/components/filtered_search_bar/constants'; diff --git a/app/assets/javascripts/issues_list/constants.js b/app/assets/javascripts/issues/list/constants.js index 4a380848b4f..4a380848b4f 100644 --- a/app/assets/javascripts/issues_list/constants.js +++ b/app/assets/javascripts/issues/list/constants.js diff --git a/app/assets/javascripts/issues_list/eventhub.js b/app/assets/javascripts/issues/list/eventhub.js index e31806ad199..e31806ad199 100644 --- a/app/assets/javascripts/issues_list/eventhub.js +++ b/app/assets/javascripts/issues/list/eventhub.js diff --git a/app/assets/javascripts/issues_list/index.js b/app/assets/javascripts/issues/list/index.js index 30c227619e0..01cc82ed8fd 100644 --- a/app/assets/javascripts/issues_list/index.js +++ b/app/assets/javascripts/issues/list/index.js @@ -1,8 +1,8 @@ import produce from 'immer'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; -import getIssuesQuery from 'ee_else_ce/issues_list/queries/get_issues.query.graphql'; -import IssuesListApp from 'ee_else_ce/issues_list/components/issues_list_app.vue'; +import getIssuesQuery from 'ee_else_ce/issues/list/queries/get_issues.query.graphql'; +import IssuesListApp from 'ee_else_ce/issues/list/components/issues_list_app.vue'; import createDefaultClient from '~/lib/graphql'; import { parseBoolean } from '~/lib/utils/common_utils'; import JiraIssuesImportStatusRoot from './components/jira_issues_import_status_app.vue'; diff --git a/app/assets/javascripts/issues_list/queries/get_issues.query.graphql b/app/assets/javascripts/issues/list/queries/get_issues.query.graphql index be8deb3fe97..be8deb3fe97 100644 --- a/app/assets/javascripts/issues_list/queries/get_issues.query.graphql +++ b/app/assets/javascripts/issues/list/queries/get_issues.query.graphql diff --git a/app/assets/javascripts/issues_list/queries/get_issues_counts.query.graphql b/app/assets/javascripts/issues/list/queries/get_issues_counts.query.graphql index 1a345fd2877..1a345fd2877 100644 --- a/app/assets/javascripts/issues_list/queries/get_issues_counts.query.graphql +++ b/app/assets/javascripts/issues/list/queries/get_issues_counts.query.graphql diff --git a/app/assets/javascripts/issues_list/queries/get_issues_list_details.query.graphql b/app/assets/javascripts/issues/list/queries/get_issues_list_details.query.graphql index a53dba8c7c8..a53dba8c7c8 100644 --- a/app/assets/javascripts/issues_list/queries/get_issues_list_details.query.graphql +++ b/app/assets/javascripts/issues/list/queries/get_issues_list_details.query.graphql diff --git a/app/assets/javascripts/issues_list/queries/issue.fragment.graphql b/app/assets/javascripts/issues/list/queries/issue.fragment.graphql index 07dae3fd756..07dae3fd756 100644 --- a/app/assets/javascripts/issues_list/queries/issue.fragment.graphql +++ b/app/assets/javascripts/issues/list/queries/issue.fragment.graphql diff --git a/app/assets/javascripts/issues_list/queries/label.fragment.graphql b/app/assets/javascripts/issues/list/queries/label.fragment.graphql index bb1d8f1ac9b..bb1d8f1ac9b 100644 --- a/app/assets/javascripts/issues_list/queries/label.fragment.graphql +++ b/app/assets/javascripts/issues/list/queries/label.fragment.graphql diff --git a/app/assets/javascripts/issues_list/queries/milestone.fragment.graphql b/app/assets/javascripts/issues/list/queries/milestone.fragment.graphql index 3cdf69bf585..3cdf69bf585 100644 --- a/app/assets/javascripts/issues_list/queries/milestone.fragment.graphql +++ b/app/assets/javascripts/issues/list/queries/milestone.fragment.graphql diff --git a/app/assets/javascripts/issues_list/queries/reorder_issues.mutation.graphql b/app/assets/javascripts/issues/list/queries/reorder_issues.mutation.graphql index 160026a4742..160026a4742 100644 --- a/app/assets/javascripts/issues_list/queries/reorder_issues.mutation.graphql +++ b/app/assets/javascripts/issues/list/queries/reorder_issues.mutation.graphql diff --git a/app/assets/javascripts/issues_list/queries/search_labels.query.graphql b/app/assets/javascripts/issues/list/queries/search_labels.query.graphql index 44b57317161..44b57317161 100644 --- a/app/assets/javascripts/issues_list/queries/search_labels.query.graphql +++ b/app/assets/javascripts/issues/list/queries/search_labels.query.graphql diff --git a/app/assets/javascripts/issues_list/queries/search_milestones.query.graphql b/app/assets/javascripts/issues/list/queries/search_milestones.query.graphql index e7eb08104a6..e7eb08104a6 100644 --- a/app/assets/javascripts/issues_list/queries/search_milestones.query.graphql +++ b/app/assets/javascripts/issues/list/queries/search_milestones.query.graphql diff --git a/app/assets/javascripts/issues_list/queries/search_projects.query.graphql b/app/assets/javascripts/issues/list/queries/search_projects.query.graphql index bd2f9bc2340..bd2f9bc2340 100644 --- a/app/assets/javascripts/issues_list/queries/search_projects.query.graphql +++ b/app/assets/javascripts/issues/list/queries/search_projects.query.graphql diff --git a/app/assets/javascripts/issues_list/queries/search_users.query.graphql b/app/assets/javascripts/issues/list/queries/search_users.query.graphql index 92517ad35d0..92517ad35d0 100644 --- a/app/assets/javascripts/issues_list/queries/search_users.query.graphql +++ b/app/assets/javascripts/issues/list/queries/search_users.query.graphql diff --git a/app/assets/javascripts/issues_list/queries/user.fragment.graphql b/app/assets/javascripts/issues/list/queries/user.fragment.graphql index 3e5bc0f7b93..3e5bc0f7b93 100644 --- a/app/assets/javascripts/issues_list/queries/user.fragment.graphql +++ b/app/assets/javascripts/issues/list/queries/user.fragment.graphql diff --git a/app/assets/javascripts/issues_list/utils.js b/app/assets/javascripts/issues/list/utils.js index 99946e4e851..c64925edc28 100644 --- a/app/assets/javascripts/issues_list/utils.js +++ b/app/assets/javascripts/issues/list/utils.js @@ -36,7 +36,7 @@ import { urlSortParams, WEIGHT_ASC, WEIGHT_DESC, -} from '~/issues_list/constants'; +} from '~/issues/list/constants'; import { isPositiveInteger } from '~/lib/utils/number_utils'; import { __ } from '~/locale'; import { diff --git a/app/assets/javascripts/pages/groups/issues/index.js b/app/assets/javascripts/pages/groups/issues/index.js index b68e52d8ab1..725c38defc3 100644 --- a/app/assets/javascripts/pages/groups/issues/index.js +++ b/app/assets/javascripts/pages/groups/issues/index.js @@ -1,6 +1,6 @@ import IssuableFilteredSearchTokenKeys from 'ee_else_ce/filtered_search/issuable_filtered_search_token_keys'; import { initBulkUpdateSidebar } from '~/issuable/bulk_update_sidebar'; -import { mountIssuesListApp } from '~/issues_list'; +import { mountIssuesListApp } from '~/issues/list'; import initManualOrdering from '~/issues/manual_ordering'; import { FILTERED_SEARCH } from '~/filtered_search/constants'; import initFilteredSearch from '~/pages/search/init_filtered_search'; diff --git a/app/assets/javascripts/pages/projects/issues/index/index.js b/app/assets/javascripts/pages/projects/issues/index/index.js index c8fdcfe0502..44b1d5277d1 100644 --- a/app/assets/javascripts/pages/projects/issues/index/index.js +++ b/app/assets/javascripts/pages/projects/issues/index/index.js @@ -2,7 +2,7 @@ import IssuableFilteredSearchTokenKeys from 'ee_else_ce/filtered_search/issuable import ShortcutsNavigation from '~/behaviors/shortcuts/shortcuts_navigation'; import { initCsvImportExportButtons, initIssuableByEmail } from '~/issuable'; import { initBulkUpdateSidebar, initIssueStatusSelect } from '~/issuable/bulk_update_sidebar'; -import { mountIssuesListApp, mountJiraIssuesListApp } from '~/issues_list'; +import { mountIssuesListApp, mountJiraIssuesListApp } from '~/issues/list'; import initManualOrdering from '~/issues/manual_ordering'; import { FILTERED_SEARCH } from '~/filtered_search/constants'; import { ISSUABLE_INDEX } from '~/issuable/constants'; diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue index 677c50ed930..2e3a02b1712 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue @@ -1,5 +1,6 @@ <script> import { GlButton, GlLoadingIcon } from '@gitlab/ui'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import ciIcon from '../../vue_shared/components/ci_icon.vue'; export default { @@ -8,6 +9,7 @@ export default { GlButton, GlLoadingIcon, }, + mixins: [glFeatureFlagMixin()], props: { status: { type: String, @@ -42,7 +44,7 @@ export default { </div> <gl-button - v-if="showDisabledButton" + v-if="!glFeatures.restructuredMrWidget && showDisabledButton" category="primary" variant="success" data-testid="disabled-merge-button" diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/merge_checks_failed.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/merge_checks_failed.vue index ce572f8b0bf..574ff8b3b2f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/merge_checks_failed.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/merge_checks_failed.vue @@ -1,20 +1,16 @@ <script> -import { GlButton } from '@gitlab/ui'; import { s__ } from '~/locale'; -import notesEventHub from '~/notes/event_hub'; import StatusIcon from '../mr_widget_status_icon.vue'; export default { i18n: { - pipelineFailed: s__( - 'mrWidget|The pipeline for this merge request did not complete. Push a new commit to fix the failure.', - ), approvalNeeded: s__('mrWidget|Merge blocked: this merge request must be approved.'), - unresolvedDiscussions: s__('mrWidget|Merge blocked: all threads must be resolved.'), + blockingMergeRequests: s__( + 'mrWidget|Merge blocked: you can only merge once above items are resolved.', + ), }, components: { StatusIcon, - GlButton, }, props: { mr: { @@ -24,22 +20,15 @@ export default { }, computed: { failedText() { - if (this.mr.isPipelineFailed) { - return this.$options.i18n.pipelineFailed; - } else if (this.mr.approvals && !this.mr.isApproved) { + if (this.mr.approvals && !this.mr.isApproved) { return this.$options.i18n.approvalNeeded; - } else if (this.mr.hasMergeableDiscussionsState) { - return this.$options.i18n.unresolvedDiscussions; + } else if (this.mr.blockingMergeRequests?.total_count > 0) { + return this.$options.i18n.blockingMergeRequests; } return null; }, }, - methods: { - jumpToFirstUnresolvedDiscussion() { - notesEventHub.$emit('jumpToFirstUnresolvedDiscussion'); - }, - }, }; </script> @@ -48,28 +37,6 @@ export default { <status-icon status="warning" /> <p class="media-body gl-m-0! gl-font-weight-bold gl-text-black-normal!"> {{ failedText }} - <template v-if="failedText == $options.i18n.unresolvedDiscussions"> - <gl-button - class="gl-ml-3" - size="small" - variant="confirm" - data-testid="jumpToUnresolved" - @click="jumpToFirstUnresolvedDiscussion" - > - {{ s__('mrWidget|Jump to first unresolved thread') }} - </gl-button> - <gl-button - v-if="mr.createIssueToResolveDiscussionsPath" - :href="mr.createIssueToResolveDiscussionsPath" - class="gl-ml-3" - size="small" - variant="confirm" - category="secondary" - data-testid="resolveIssue" - > - {{ s__('mrWidget|Create issue to resolve all threads') }} - </gl-button> - </template> </p> </div> </template> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_archived.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_archived.vue index 13b1e49f44e..071920856a8 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_archived.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_archived.vue @@ -1,25 +1,22 @@ <script> -import { GlButton } from '@gitlab/ui'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import statusIcon from '../mr_widget_status_icon.vue'; export default { name: 'MRWidgetArchived', components: { - GlButton, statusIcon, }, + mixins: [glFeatureFlagMixin()], }; </script> <template> <div class="mr-widget-body media"> <div class="space-children"> - <status-icon status="warning" /> - <gl-button category="secondary" variant="success" :disabled="true"> - {{ s__('mrWidget|Merge') }} - </gl-button> + <status-icon status="warning" show-disabled-button /> </div> <div class="media-body"> - <span class="bold"> + <span :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" class="bold"> {{ s__('mrWidget|Merge unavailable: merge requests are read-only on archived projects.') }} </span> </div> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.vue index 10b93d7849f..fd42fa0421f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_checking.vue @@ -12,9 +12,11 @@ export default { </script> <template> <div class="mr-widget-body media"> - <status-icon :show-disabled-button="!glFeatures.restructuredMrWidget" status="loading" /> + <status-icon :show-disabled-button="true" status="loading" /> <div class="media-body space-children"> - <span class="bold"> {{ s__('mrWidget|Checking if merge request can be merged…') }} </span> + <span :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" class="bold"> + {{ s__('mrWidget|Checking if merge request can be merged…') }} + </span> </div> </div> </template> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue index 7a002d41ac0..a2c9cfe53cc 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue @@ -109,14 +109,18 @@ export default { </gl-skeleton-loader> </div> <div v-else class="media-body space-children gl-display-flex gl-align-items-center"> - <span v-if="shouldBeRebased" class="bold"> + <span + v-if="shouldBeRebased" + :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" + class="bold" + > {{ s__(`mrWidget|Merge blocked: fast-forward merge is not possible. To merge this request, first rebase locally.`) }} </span> <template v-else> - <span class="bold"> + <span :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" class="bold"> {{ s__('mrWidget|Merge blocked: merge conflicts must be resolved.') }} <span v-if="!canMerge"> {{ @@ -129,6 +133,7 @@ export default { <gl-button v-if="showResolveButton" :href="mr.conflictResolutionPath" + :size="glFeatures.restructuredMrWidget ? 'small' : 'medium'" data-testid="resolve-conflicts-button" > {{ s__('mrWidget|Resolve conflicts') }} @@ -136,6 +141,7 @@ export default { <gl-button v-if="canMerge" v-gl-modal-directive="'modal-merge-info'" + :size="glFeatures.restructuredMrWidget ? 'small' : 'medium'" data-testid="merge-locally-button" > {{ s__('mrWidget|Merge locally') }} diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_missing_branch.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_missing_branch.vue index f91350d4a82..5b03eda2eac 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_missing_branch.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_missing_branch.vue @@ -74,10 +74,21 @@ export default { <status-icon :show-disabled-button="true" status="warning" /> <div class="media-body space-children"> - <span class="bold js-branch-text"> + <span + :class="{ + 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget, + }" + class="bold js-branch-text" + > <span class="capitalize" data-testid="missingBranchName"> {{ missingBranchName }} </span> {{ s__('mrWidget|branch does not exist.') }} {{ missingBranchNameMessage }} - <gl-icon v-gl-tooltip :title="message" :aria-label="message" name="question-o" /> + <gl-icon + v-gl-tooltip + :title="message" + :aria-label="message" + name="question-o" + class="gl-text-blue-600 gl-cursor-pointer" + /> </span> </div> </div> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_pipeline_blocked.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_pipeline_blocked.vue index 68ffca9cd68..34c5a2ff2c8 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_pipeline_blocked.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_pipeline_blocked.vue @@ -1,4 +1,5 @@ <script> +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import StatusIcon from '../mr_widget_status_icon.vue'; export default { @@ -6,13 +7,14 @@ export default { components: { StatusIcon, }, + mixins: [glFeatureFlagMixin()], }; </script> <template> <div class="mr-widget-body media"> <status-icon :show-disabled-button="true" status="warning" /> <div class="media-body space-children"> - <span class="bold"> + <span :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" class="bold"> {{ s__( `mrWidget|Merge blocked: pipeline must succeed. It's waiting for a manual action to continue.`, diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue index 01e8303f513..886fab8634c 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue @@ -152,12 +152,14 @@ export default { <div class="rebase-state-find-class-convention media media-body space-children"> <span v-if="rebaseInProgress || isMakingRequest" + :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" class="gl-font-weight-bold" data-testid="rebase-message" >{{ __('Rebase in progress') }}</span > <span v-if="!rebaseInProgress && !canPushToSourceBranch" + :class="{ 'gl-text-body!': glFeatures.restructuredMrWidget }" class="gl-font-weight-bold gl-ml-0!" data-testid="rebase-message" >{{ fastForwardMergeText }}</span @@ -167,6 +169,7 @@ export default { class="accept-merge-holder clearfix js-toggle-container accept-action media space-children" > <gl-button + v-if="!glFeatures.restructuredMrWidget" :loading="isMakingRequest" variant="confirm" data-qa-selector="mr_rebase_button" @@ -176,6 +179,7 @@ export default { </gl-button> <span v-if="!rebasingError" + :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" class="gl-font-weight-bold" data-testid="rebase-message" data-qa-selector="no_fast_forward_message_content" @@ -186,6 +190,17 @@ export default { <span v-else class="gl-font-weight-bold danger" data-testid="rebase-message">{{ rebasingError }}</span> + <gl-button + v-if="glFeatures.restructuredMrWidget" + :loading="isMakingRequest" + variant="confirm" + size="small" + data-qa-selector="mr_rebase_button" + class="gl-ml-3!" + @click="rebase" + > + {{ __('Rebase') }} + </gl-button> </div> </div> </template> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/pipeline_failed.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/pipeline_failed.vue index b5d2f91c637..d88dad2e086 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/pipeline_failed.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/pipeline_failed.vue @@ -1,6 +1,7 @@ <script> import { GlLink, GlSprintf } from '@gitlab/ui'; import { helpPagePath } from '~/helpers/help_page_helper'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { s__ } from '~/locale'; import statusIcon from '../mr_widget_status_icon.vue'; @@ -11,6 +12,7 @@ export default { GlSprintf, statusIcon, }, + mixins: [glFeatureFlagMixin()], computed: { troubleshootingDocsPath() { return helpPagePath('ci/troubleshooting', { anchor: 'merge-request-status-messages' }); @@ -28,7 +30,7 @@ export default { <div class="mr-widget-body media"> <status-icon :show-disabled-button="true" status="warning" /> <div class="media-body space-children"> - <span class="bold"> + <span :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" class="bold"> <gl-sprintf :message="$options.i18n.failedMessage"> <template #link="{ content }"> <gl-link :href="troubleshootingDocsPath" target="_blank"> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index 8830128b7d6..06ce312bd4c 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -529,7 +529,7 @@ export default { <template> <div :class="{ - 'gl-border-t-1 gl-border-t-solid gl-border-gray-100 gl-bg-gray-10 gl-pl-7': + 'gl-border-t-1 gl-border-t-solid gl-border-gray-100 gl-bg-gray-10 gl-pl-7 gl-rounded-bottom-left-base gl-rounded-bottom-right-base': glFeatures.restructuredMrWidget, }" > diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/sha_mismatch.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/sha_mismatch.vue index 7eeba8d8f89..b1fbe150fcf 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/sha_mismatch.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/sha_mismatch.vue @@ -1,5 +1,6 @@ <script> import { GlButton } from '@gitlab/ui'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { I18N_SHA_MISMATCH } from '../../i18n'; import statusIcon from '../mr_widget_status_icon.vue'; @@ -12,6 +13,7 @@ export default { i18n: { I18N_SHA_MISMATCH, }, + mixins: [glFeatureFlagMixin()], props: { mr: { type: Object, @@ -25,7 +27,11 @@ export default { <div class="mr-widget-body media"> <status-icon :show-disabled-button="false" status="warning" /> <div class="media-body"> - <span class="gl-font-weight-bold" data-qa-selector="head_mismatch_content"> + <span + :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" + class="gl-font-weight-bold" + data-qa-selector="head_mismatch_content" + > {{ $options.i18n.I18N_SHA_MISMATCH.warningMessage }} </span> <gl-button diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/unresolved_discussions.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/unresolved_discussions.vue index 69e4df0ca11..8cf6383c26a 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/unresolved_discussions.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/unresolved_discussions.vue @@ -1,5 +1,6 @@ <script> import { GlButton } from '@gitlab/ui'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import notesEventHub from '~/notes/event_hub'; import statusIcon from '../mr_widget_status_icon.vue'; @@ -9,6 +10,7 @@ export default { statusIcon, GlButton, }, + mixins: [glFeatureFlagMixin()], props: { mr: { type: Object, @@ -25,16 +27,24 @@ export default { <template> <div class="mr-widget-body media gl-flex-wrap"> - <status-icon :show-disabled-button="true" status="warning" /> + <status-icon show-disabled-button status="warning" /> <div class="media-body"> - <span class="gl-ml-3 gl-font-weight-bold gl-display-block gl-w-100">{{ - s__('mrWidget|Merge blocked: all threads must be resolved.') - }}</span> + <span + :class="{ + 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget, + 'gl-display-block': !glFeatures.restructuredMrWidget, + }" + class="gl-ml-3 gl-font-weight-bold gl-w-100" + > + {{ s__('mrWidget|Merge blocked: all threads must be resolved.') }} + </span> <gl-button data-testid="jump-to-first" class="gl-ml-3" size="small" - icon="comment-next" + :icon="glFeatures.restructuredMrWidget ? undefined : 'comment-next'" + :variant="glFeatures.restructuredMrWidget && 'confirm'" + :category="glFeatures.restructuredMrWidget && 'secondary'" @click="jumpToFirstUnresolvedDiscussion" > {{ s__('mrWidget|Jump to first unresolved thread') }} @@ -44,7 +54,7 @@ export default { :href="mr.createIssueToResolveDiscussionsPath" class="js-create-issue gl-ml-3" size="small" - icon="issue-new" + :icon="glFeatures.restructuredMrWidget ? undefined : 'issue-new'" > {{ s__('mrWidget|Create issue to resolve all threads') }} </gl-button> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue index ba831a33b73..e0e19094c40 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue @@ -166,7 +166,10 @@ export default { <status-icon :show-disabled-button="canUpdate" status="warning" /> <div class="media-body"> <div class="float-left"> - <span class="gl-font-weight-bold"> + <span + :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }" + class="gl-font-weight-bold" + > {{ __("Merge blocked: merge request must be marked as ready. It's still marked as draft.") }} diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 57af869a0ba..d95982eb74c 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -357,15 +357,13 @@ export default class MergeRequestStore { setApprovals(data) { this.approvals = data; this.isApproved = data.approved || false; + + this.setState(); } + // eslint-disable-next-line class-methods-use-this get hasMergeChecksFailed() { - if (!window.gon?.features?.restructuredMrWidget) return false; - - return ( - this.hasMergeableDiscussionsState || - (this.onlyAllowMergeIfPipelineSucceeds && this.isPipelineFailed) - ); + return false; } // Because the state machine doesn't yet handle every state and transition, diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index 16b95d2a2b9..9f45160d3a8 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -41,6 +41,11 @@ class ProjectHook < WebHook super.merge(project: project) end + override :parent + def parent + project + end + private override :web_hooks_disable_failed? diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb index 1a466b333a5..80e167b350b 100644 --- a/app/models/hooks/service_hook.rb +++ b/app/models/hooks/service_hook.rb @@ -2,6 +2,7 @@ class ServiceHook < WebHook include Presentable + extend ::Gitlab::Utils::Override belongs_to :integration, foreign_key: :service_id validates :integration, presence: true @@ -9,4 +10,7 @@ class ServiceHook < WebHook def execute(data, hook_name = 'service_hook') super(data, hook_name) end + + override :parent + delegate :parent, to: :integration end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index e8a55abfc8f..3320c13e87b 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -122,6 +122,11 @@ class WebHook < ApplicationRecord nil end + # Returns the associated Project or Group for the WebHook if one exists. + # Overridden by inheriting classes. + def parent + end + # Custom attributes to be included in the worker context. def application_context { related_class: type } diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 79bdf34392f..33e34ec41e2 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -26,7 +26,6 @@ class WebHookService end REQUEST_BODY_SIZE_LIMIT = 25.megabytes - GITLAB_EVENT_HEADER = 'X-Gitlab-Event' attr_accessor :hook, :data, :hook_name, :request_options attr_reader :uniqueness_token @@ -50,6 +49,10 @@ class WebHookService def execute return { status: :error, message: 'Hook disabled' } unless hook.executable? + log_recursion_limit if recursion_blocked? + + Gitlab::WebHooks::RecursionDetection.register!(hook) + start_time = Gitlab::Metrics::System.monotonic_time response = if parsed_url.userinfo.blank? @@ -95,6 +98,10 @@ class WebHookService Gitlab::ApplicationContext.with_context(hook.application_context) do break log_rate_limit if rate_limited? + log_recursion_limit if recursion_blocked? + + data[:_gitlab_recursion_detection_request_uuid] = Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid + WebHookWorker.perform_async(hook.id, data, hook_name) end end @@ -108,7 +115,7 @@ class WebHookService def make_request(url, basic_auth = false) Gitlab::HTTP.post(url, body: Gitlab::Json::LimitedEncoder.encode(data, limit: REQUEST_BODY_SIZE_LIMIT), - headers: build_headers(hook_name), + headers: build_headers, verify: hook.enable_ssl_verification, basic_auth: basic_auth, **request_options) @@ -129,7 +136,7 @@ class WebHookService trigger: trigger, url: url, execution_duration: execution_duration, - request_headers: build_headers(hook_name), + request_headers: build_headers, request_data: request_data, response_headers: format_response_headers(response), response_body: safe_response_body(response), @@ -151,15 +158,16 @@ class WebHookService end end - def build_headers(hook_name) + def build_headers @headers ||= begin - { + headers = { 'Content-Type' => 'application/json', 'User-Agent' => "GitLab/#{Gitlab::VERSION}", - GITLAB_EVENT_HEADER => self.class.hook_to_event(hook_name) - }.tap do |hash| - hash['X-Gitlab-Token'] = Gitlab::Utils.remove_line_breaks(hook.token) if hook.token.present? - end + Gitlab::WebHooks::GITLAB_EVENT_HEADER => self.class.hook_to_event(hook_name) + } + + headers['X-Gitlab-Token'] = Gitlab::Utils.remove_line_breaks(hook.token) if hook.token.present? + headers.merge!(Gitlab::WebHooks::RecursionDetection.header(hook)) end end @@ -186,6 +194,10 @@ class WebHookService ) end + def recursion_blocked? + Gitlab::WebHooks::RecursionDetection.block?(hook) + end + def rate_limit @rate_limit ||= hook.rate_limit end @@ -199,4 +211,15 @@ class WebHookService **Gitlab::ApplicationContext.current ) end + + def log_recursion_limit + Gitlab::AuthLogger.error( + message: 'Webhook recursion detected and will be blocked in future', + hook_id: hook.id, + hook_type: hook.type, + hook_name: hook_name, + recursion_detection: ::Gitlab::WebHooks::RecursionDetection.to_log(hook), + **Gitlab::ApplicationContext.current + ) + end end diff --git a/app/workers/web_hook_worker.rb b/app/workers/web_hook_worker.rb index 5b4567dde29..952ac94d5e6 100644 --- a/app/workers/web_hook_worker.rb +++ b/app/workers/web_hook_worker.rb @@ -13,11 +13,21 @@ class WebHookWorker worker_has_external_dependencies! - def perform(hook_id, data, hook_name) + # Webhook recursion detection properties are passed through the `data` arg. + # This will be migrated to the `params` arg over the next few releases. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/347389. + def perform(hook_id, data, hook_name, params = {}) hook = WebHook.find_by_id(hook_id) return unless hook data = data.with_indifferent_access + + # Before executing the hook, reapply any recursion detection UUID that was + # initially present in the request header so the hook can pass this same header + # value in its request. + recursion_detection_uuid = data.delete(:_gitlab_recursion_detection_request_uuid) + Gitlab::WebHooks::RecursionDetection.set_request_uuid(recursion_detection_uuid) + WebHookService.new(hook, data, hook_name, jid).execute end end diff --git a/config/feature_flags/development/webhook_recursion_detection.yml b/config/feature_flags/development/webhook_recursion_detection.yml new file mode 100644 index 00000000000..8ef0bf81491 --- /dev/null +++ b/config/feature_flags/development/webhook_recursion_detection.yml @@ -0,0 +1,8 @@ +--- +name: webhook_recursion_detection +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75821 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/349845 +milestone: '14.7' +type: development +group: group::integrations +default_enabled: false diff --git a/config/initializers/webhook_recursion_detection.rb b/config/initializers/webhook_recursion_detection.rb new file mode 100644 index 00000000000..b345c005bac --- /dev/null +++ b/config/initializers/webhook_recursion_detection.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +Rails.application.configure do |config| + config.middleware.insert_after RequestStore::Middleware, Gitlab::Middleware::WebhookRecursionDetection +end diff --git a/doc/administration/operations/moving_repositories.md b/doc/administration/operations/moving_repositories.md index 84e6dca1f2b..f0eb5792a96 100644 --- a/doc/administration/operations/moving_repositories.md +++ b/doc/administration/operations/moving_repositories.md @@ -184,8 +184,8 @@ Each of the approaches we list can or does overwrite data in the target director ### Recommended approach in all cases -The GitLab [backup and restore capability](../../raketasks/backup_restore.md) should be used. Git -repositories are accessed, managed, and stored on GitLab servers by Gitaly as a database. Data loss +For either Gitaly or Gitaly Cluster targets, the GitLab [backup and restore capability](../../raketasks/backup_restore.md) +should be used. Git repositories are accessed, managed, and stored on GitLab servers by Gitaly as a database. Data loss can result from directly accessing and copying Gitaly's files using tools like `rsync`. - From GitLab 13.3, backup performance can be improved by @@ -193,13 +193,15 @@ can result from directly accessing and copying Gitaly's files using tools like ` - Backups can be created of just the repositories using the [skip feature](../../raketasks/backup_restore.md#excluding-specific-directories-from-the-backup). +No other method works for Gitaly Cluster targets. + ### Target directory is empty: use a `tar` pipe -If the target directory `/mnt/gitlab/repositories` is empty the -simplest thing to do is to use a `tar` pipe. This method has low -overhead and `tar` is almost always already installed on your system. -However, it is not possible to resume an interrupted `tar` pipe: if -that happens then all data must be copied again. +For Gitaly targets (use [recommended approach](#recommended-approach-in-all-cases) for Gitaly Cluster targets), if the +target directory `/mnt/gitlab/repositories` is empty the simplest thing to do is to use a `tar` pipe. This method has +low overhead and `tar` is almost always already installed on your system. + +However, it is not possible to resume an interrupted `tar` pipe; if that happens then all data must be copied again. ```shell sudo -u git sh -c 'tar -C /var/opt/gitlab/git-data/repositories -cf - -- . |\ @@ -210,9 +212,9 @@ If you want to see progress, replace `-xf` with `-xvf`. #### `tar` pipe to another server -You can also use a `tar` pipe to copy data to another server. If your -`git` user has SSH access to the new server as `git@newserver`, you -can pipe the data through SSH. +For Gitaly targets (use [recommended approach](#recommended-approach-in-all-cases) for Gitaly Cluster targets), you can +also use a `tar` pipe to copy data to another server. If your `git` user has SSH access to the new server as +`git@newserver`, you can pipe the data through SSH. ```shell sudo -u git sh -c 'tar -C /var/opt/gitlab/git-data/repositories -cf - -- . |\ @@ -228,11 +230,11 @@ WARNING: Using `rsync` to migrate Git data can cause data loss and repository corruption. [These instructions are being reviewed](https://gitlab.com/gitlab-org/gitlab/-/issues/270422). -If the target directory already contains a partial / outdated copy -of the repositories it may be wasteful to copy all the data again -with `tar`. In this scenario it is better to use `rsync`. This utility -is either already installed on your system, or installable -by using `apt` or `yum`. +If the target directory already contains a partial or outdated copy of the repositories it may be wasteful to copy all +the data again with `tar`. In this scenario it is better to use `rsync` for Gitaly targets (use +[recommended approach](#recommended-approach-in-all-cases) for Gitaly Cluster targets). + +This utility is either already installed on your system, or installable using `apt` or `yum`. ```shell sudo -u git sh -c 'rsync -a --delete /var/opt/gitlab/git-data/repositories/. \ @@ -249,8 +251,9 @@ WARNING: Using `rsync` to migrate Git data can cause data loss and repository corruption. [These instructions are being reviewed](https://gitlab.com/gitlab-org/gitlab/-/issues/270422). -If the `git` user on your source system has SSH access to the target -server you can send the repositories over the network with `rsync`. +For Gitaly targets (use [recommended approach](#recommended-approach-in-all-cases) for Gitaly Cluster targets), if the +`git` user on your source system has SSH access to the target server you can send the repositories over the network with +`rsync`. ```shell sudo -u git sh -c 'rsync -a --delete /var/opt/gitlab/git-data/repositories/. \ @@ -269,17 +272,18 @@ Every time you start an `rsync` job it must: - Inspect all files in the target directory. - Decide whether or not to copy files. -If the source or target directory -has many contents, this startup phase of `rsync` can become a burden -for your GitLab server. You can reduce the workload of `rsync` by dividing its -work in smaller pieces, and sync one repository at a time. +If the source or target directory has many contents, this startup phase of `rsync` can become a burden for your GitLab +server. You can reduce the workload of `rsync` by dividing its work into smaller pieces, and sync one repository at a +time. In addition to `rsync` we use [GNU Parallel](http://www.gnu.org/software/parallel/). This utility is not included in GitLab, so you must install it yourself with `apt` or `yum`. -This process does not clean up repositories at the target location that no -longer exist at the source. +This process: + +- Doesn't clean up repositories at the target location that no longer exist at the source. +- Only works for Gitaly targets. Use [recommended approach](#recommended-approach-in-all-cases) for Gitaly Cluster targets. #### Parallel `rsync` for all repositories known to GitLab diff --git a/doc/user/application_security/dast/index.md b/doc/user/application_security/dast/index.md index b65747969c9..3cf842f0de6 100644 --- a/doc/user/application_security/dast/index.md +++ b/doc/user/application_security/dast/index.md @@ -638,7 +638,7 @@ These CI/CD variables are specific to DAST. They can be used to customize the be | `DAST_XML_REPORT` | string | The filename of the XML report written at the end of a scan. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12652) in GitLab 13.1. | | `DAST_WEBSITE` <sup>1</sup> | URL | The URL of the website to scan. The variable `DAST_API_OPENAPI` must be specified if this is omitted. | | `DAST_ZAP_CLI_OPTIONS` | string | ZAP server command-line options. For example, `-Xmx3072m` would set the Java maximum memory allocation pool size. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12652) in GitLab 13.1. | -| `DAST_ZAP_LOG_CONFIGURATION` | string | Set to a semicolon-separated list of additional log4j properties for the ZAP Server. Example: `log4j.logger.org.parosproxy.paros.network.HttpSender=DEBUG;log4j.logger.com.crawljax=DEBUG` | +| `DAST_ZAP_LOG_CONFIGURATION` | string | Set to a semicolon-separated list of additional log4j properties for the ZAP Server. Example: `logger.httpsender.name=org.parosproxy.paros.network.HttpSender;logger.httpsender.level=debug;logger.sitemap.name=org.parosproxy.paros.model.SiteMap;logger.sitemap.level=debug;` | | `SECURE_ANALYZERS_PREFIX` | URL | Set the Docker registry base address from which to download the analyzer. | 1. Available to an on-demand DAST scan. diff --git a/doc/user/project/members/index.md b/doc/user/project/members/index.md index 15ebd60f246..2dc29f5d725 100644 --- a/doc/user/project/members/index.md +++ b/doc/user/project/members/index.md @@ -211,7 +211,7 @@ Instead of adding users one by one, you can [share a project with an entire grou > - Enabled on GitLab.com. > - Recommended for production use. > - Replaces the existing form with buttons to open a modal window. -> - To use in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-modal-window). **(FREE SELF)** +> - To use in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-modal-window). WARNING: This feature might not be available to you. Check the **version history** note above for details. diff --git a/doc/user/project/members/share_project_with_groups.md b/doc/user/project/members/share_project_with_groups.md index 4c96c4d9f56..4e3bae2dc30 100644 --- a/doc/user/project/members/share_project_with_groups.md +++ b/doc/user/project/members/share_project_with_groups.md @@ -52,7 +52,7 @@ Administrators can share projects with any group in the system. > - Enabled on GitLab.com. > - Recommended for production use. > - Replaces the existing form with buttons to open a modal window. -> - To use in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-modal-window). **(FREE SELF)** +> - To use in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-modal-window). WARNING: This feature might not be available to you. Check the **version history** note above for details. diff --git a/lib/api/ci/triggers.rb b/lib/api/ci/triggers.rb index 6a2b16e1568..ae89b475ef8 100644 --- a/lib/api/ci/triggers.rb +++ b/lib/api/ci/triggers.rb @@ -5,7 +5,7 @@ module API class Triggers < ::API::Base include PaginationParams - HTTP_GITLAB_EVENT_HEADER = "HTTP_#{WebHookService::GITLAB_EVENT_HEADER}".underscore.upcase + HTTP_GITLAB_EVENT_HEADER = "HTTP_#{::Gitlab::WebHooks::GITLAB_EVENT_HEADER}".underscore.upcase feature_category :continuous_integration diff --git a/lib/backup/gitaly_backup.rb b/lib/backup/gitaly_backup.rb index b104beed39c..8ac09e94004 100644 --- a/lib/backup/gitaly_backup.rb +++ b/lib/backup/gitaly_backup.rb @@ -2,11 +2,17 @@ module Backup # Backup and restores repositories using gitaly-backup + # + # gitaly-backup can work in parallel and accepts a list of repositories + # through input pipe using a specific json format for both backup and restore class GitalyBackup - def initialize(progress, parallel: nil, parallel_storage: nil) + # @param [StringIO] progress IO interface to output progress + # @param [Integer] max_parallelism max parallelism when running backups + # @param [Integer] storage_parallelism max parallelism per storage (is affected by max_parallelism) + def initialize(progress, max_parallelism: nil, storage_parallelism: nil) @progress = progress - @parallel = parallel - @parallel_storage = parallel_storage + @max_parallelism = max_parallelism + @storage_parallelism = storage_parallelism end def start(type) @@ -22,20 +28,20 @@ module Backup end args = [] - args += ['-parallel', @parallel.to_s] if @parallel - args += ['-parallel-storage', @parallel_storage.to_s] if @parallel_storage + args += ['-parallel', @max_parallelism.to_s] if @max_parallelism + args += ['-parallel-storage', @storage_parallelism.to_s] if @storage_parallelism - @stdin, stdout, @thread = Open3.popen2(build_env, bin_path, command, '-path', backup_repos_path, *args) + @input_stream, stdout, @thread = Open3.popen2(build_env, bin_path, command, '-path', backup_repos_path, *args) @out_reader = Thread.new do IO.copy_stream(stdout, @progress) end end - def wait + def finish! return unless started? - @stdin.close + @input_stream.close [@thread, @out_reader].each(&:join) status = @thread.value @@ -49,12 +55,7 @@ module Backup repository = repo_type.repository_for(container) - @stdin.puts({ - storage_name: repository.storage, - relative_path: repository.relative_path, - gl_project_path: repository.gl_project_path, - always_create: repo_type.project? - }.merge(Gitlab::GitalyClient.connection_data(repository.storage)).to_json) + schedule_backup_job(repository, always_create: repo_type.project?) end def parallel_enqueue? @@ -63,6 +64,24 @@ module Backup private + # Schedule a new backup job through a non-blocking JSON based pipe protocol + # + # @see https://gitlab.com/gitlab-org/gitaly/-/blob/master/doc/gitaly-backup.md + def schedule_backup_job(repository, always_create:) + connection_params = Gitlab::GitalyClient.connection_data(repository.storage) + + json_job = { + address: connection_params['address'], + token: connection_params['token'], + storage_name: repository.storage, + relative_path: repository.relative_path, + gl_project_path: repository.gl_project_path, + always_create: always_create + }.to_json + + @input_stream.puts(json_job) + end + def build_env { 'SSL_CERT_FILE' => OpenSSL::X509::DEFAULT_CERT_FILE, diff --git a/lib/backup/gitaly_rpc_backup.rb b/lib/backup/gitaly_rpc_backup.rb index baac4eb26ca..bbd83cd2157 100644 --- a/lib/backup/gitaly_rpc_backup.rb +++ b/lib/backup/gitaly_rpc_backup.rb @@ -23,7 +23,7 @@ module Backup end end - def wait + def finish! @type = nil end diff --git a/lib/backup/repositories.rb b/lib/backup/repositories.rb index 0b5a62529b4..4c39e58c87d 100644 --- a/lib/backup/repositories.rb +++ b/lib/backup/repositories.rb @@ -40,7 +40,7 @@ module Backup raise errors.pop unless errors.empty? ensure - strategy.wait + strategy.finish! end def restore @@ -48,7 +48,7 @@ module Backup enqueue_consecutive ensure - strategy.wait + strategy.finish! cleanup_snippets_without_repositories restore_object_pools diff --git a/lib/gitlab/middleware/webhook_recursion_detection.rb b/lib/gitlab/middleware/webhook_recursion_detection.rb new file mode 100644 index 00000000000..2677445852c --- /dev/null +++ b/lib/gitlab/middleware/webhook_recursion_detection.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + module Middleware + class WebhookRecursionDetection + def initialize(app) + @app = app + end + + def call(env) + headers = ActionDispatch::Request.new(env).headers + + ::Gitlab::WebHooks::RecursionDetection.set_from_headers(headers) + + @app.call(env) + end + end + end +end diff --git a/lib/gitlab/web_hooks.rb b/lib/gitlab/web_hooks.rb new file mode 100644 index 00000000000..349c7a020cc --- /dev/null +++ b/lib/gitlab/web_hooks.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Gitlab + module WebHooks + GITLAB_EVENT_HEADER = 'X-Gitlab-Event' + end +end diff --git a/lib/gitlab/web_hooks/recursion_detection.rb b/lib/gitlab/web_hooks/recursion_detection.rb new file mode 100644 index 00000000000..34f70093dcd --- /dev/null +++ b/lib/gitlab/web_hooks/recursion_detection.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +# This module detects and blocks recursive webhook requests. +# +# Recursion can happen when a webhook has been configured to make a call +# to its own GitLab instance (i.e., its API), and during the execution of +# the call the webhook is triggered again to create an infinite loop of +# being triggered. +# +# Additionally the module blocks a webhook once the number of requests to +# the instance made by a series of webhooks triggering other webhooks reaches +# a limit. +# +# Blocking recursive webhooks allows GitLab to continue to support workflows +# that use webhooks to call the API non-recursively, or do not go on to +# trigger an unreasonable number of other webhooks. +module Gitlab + module WebHooks + module RecursionDetection + COUNT_LIMIT = 100 + TOUCH_CACHE_TTL = 30.minutes + + class << self + def set_from_headers(headers) + uuid = headers[UUID::HEADER] + + return unless uuid + + set_request_uuid(uuid) + end + + def set_request_uuid(uuid) + UUID.instance.request_uuid = uuid + end + + # Before a webhook is executed, `.register!` should be called. + # Adds the webhook ID to a cache (see `#cache_key_for_hook` for + # details of the cache). + def register!(hook) + return if disabled?(hook) + + cache_key = cache_key_for_hook(hook) + + ::Gitlab::Redis::SharedState.with do |redis| + redis.multi do + redis.sadd(cache_key, hook.id) + redis.expire(cache_key, TOUCH_CACHE_TTL) + end + end + end + + # Returns true if the webhook ID is present in the cache, or if the + # number of IDs in the cache exceeds the limit (see + # `#cache_key_for_hook` for details of the cache). + def block?(hook) + return false if disabled?(hook) + + # If a request UUID has not been set then we know the request was not + # made by a webhook, and no recursion is possible. + return false unless UUID.instance.request_uuid + + cache_key = cache_key_for_hook(hook) + + ::Gitlab::Redis::SharedState.with do |redis| + redis.sismember(cache_key, hook.id) || + redis.scard(cache_key) >= COUNT_LIMIT + end + end + + def header(hook) + UUID.instance.header(hook) + end + + def to_log(hook) + { + uuid: UUID.instance.uuid_for_hook(hook), + ids: ::Gitlab::Redis::SharedState.with { |redis| redis.smembers(cache_key_for_hook(hook)).map(&:to_i) } + } + end + + private + + def disabled?(hook) + Feature.disabled?(:webhook_recursion_detection, hook.parent) + end + + # Returns a cache key scoped to a UUID. + # + # The particular UUID will be either: + # + # - A UUID that was recycled from the request headers if the request was made by a webhook. + # - a new UUID initialized for the webhook. + # + # This means that cycles of webhooks that are triggered from other webhooks + # will share the same cache, and other webhooks will use a new cache. + def cache_key_for_hook(hook) + [:webhook_recursion_detection, UUID.instance.uuid_for_hook(hook)].join(':') + end + end + end + end +end diff --git a/lib/gitlab/web_hooks/recursion_detection/uuid.rb b/lib/gitlab/web_hooks/recursion_detection/uuid.rb new file mode 100644 index 00000000000..9c52399818d --- /dev/null +++ b/lib/gitlab/web_hooks/recursion_detection/uuid.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module Gitlab + module WebHooks + module RecursionDetection + class UUID + HEADER = "#{::Gitlab::WebHooks::GITLAB_EVENT_HEADER}-UUID" + + include Singleton + + attr_accessor :request_uuid + + def initialize + self.new_uuids_for_hooks = {} + end + + class << self + # Back the Singleton with RequestStore so it is isolated to this request. + def instance + Gitlab::SafeRequestStore[:web_hook_recursion_detection_uuid] ||= new + end + end + + # Returns a UUID, which will be either: + # + # - The UUID that was recycled from the request headers if the request was made by a webhook. + # - A new UUID initialized for the webhook. + def uuid_for_hook(hook) + request_uuid || new_uuid_for_hook(hook) + end + + def header(hook) + { HEADER => uuid_for_hook(hook) } + end + + private + + attr_accessor :new_uuids_for_hooks + + def new_uuid_for_hook(hook) + new_uuids_for_hooks[hook.id] ||= SecureRandom.uuid + end + end + end + end +end diff --git a/lib/tasks/gitlab/backup.rake b/lib/tasks/gitlab/backup.rake index 1e2462a0b93..cf922f55a7b 100644 --- a/lib/tasks/gitlab/backup.rake +++ b/lib/tasks/gitlab/backup.rake @@ -351,7 +351,7 @@ namespace :gitlab do if Feature.enabled?(:gitaly_backup, default_enabled: :yaml) max_concurrency = ENV['GITLAB_BACKUP_MAX_CONCURRENCY'].presence max_storage_concurrency = ENV['GITLAB_BACKUP_MAX_STORAGE_CONCURRENCY'].presence - Backup::GitalyBackup.new(progress, parallel: max_concurrency, parallel_storage: max_storage_concurrency) + Backup::GitalyBackup.new(progress, max_parallelism: max_concurrency, storage_parallelism: max_storage_concurrency) else Backup::GitalyRpcBackup.new(progress) end diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index 18c68615637..b01a7902bf2 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -2,6 +2,41 @@ namespace :gitlab do namespace :gitaly do + desc 'Installs gitaly for running tests within gitlab-development-kit' + task :test_install, [:dir, :storage_path, :repo] => :gitlab_environment do |t, args| + inside_gdk = Rails.env.test? && File.exist?(Rails.root.join('../GDK_ROOT')) + + if ENV['FORCE_GITALY_INSTALL'] || !inside_gdk + Rake::Task["gitlab:gitaly:install"].invoke(*args) + + next + end + + gdk_gitaly_dir = ENV.fetch('GDK_GITALY', Rails.root.join('../gitaly')) + + # Our test setup expects a git repo, so clone rather than copy + clone_repo(gdk_gitaly_dir, args.dir, clone_opts: %w[--depth 1]) unless Dir.exist?(args.dir) + + # We assume the GDK gitaly already compiled binaries + build_dir = File.join(gdk_gitaly_dir, '_build') + FileUtils.cp_r(build_dir, args.dir) + + # We assume the GDK gitaly already ran bundle install + bundle_dir = File.join(gdk_gitaly_dir, 'ruby', '.bundle') + FileUtils.cp_r(bundle_dir, File.join(args.dir, 'ruby')) + + # For completeness we copy this for gitaly's make target + ruby_bundle_file = File.join(gdk_gitaly_dir, '.ruby-bundle') + FileUtils.cp_r(ruby_bundle_file, args.dir) + + gitaly_binary = File.join(build_dir, 'bin', 'gitaly') + warn_gitaly_out_of_date!(gitaly_binary, Gitlab::GitalyClient.expected_server_version) + rescue Errno::ENOENT => e + puts "Could not copy files, did you run `gdk update`? Error: #{e.message}" + + raise + end + desc 'GitLab | Gitaly | Clone and checkout gitaly' task :clone, [:dir, :storage_path, :repo] => :gitlab_environment do |t, args| warn_user_is_not_gitlab @@ -25,6 +60,9 @@ Usage: rake "gitlab:gitaly:install[/installation/dir,/storage/path]") storage_paths = { 'default' => args.storage_path } Gitlab::SetupHelper::Gitaly.create_configuration(args.dir, storage_paths) + # In CI we run scripts/gitaly-test-build + next if ENV['CI'].present? + Dir.chdir(args.dir) do Bundler.with_original_env do env = { "RUBYOPT" => nil, "BUNDLE_GEMFILE" => nil } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 98356de7376..fbcc5d5a82e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -42529,6 +42529,9 @@ msgstr "" msgid "mrWidget|Merge blocked: this merge request must be approved." msgstr "" +msgid "mrWidget|Merge blocked: you can only merge once above items are resolved." +msgstr "" + msgid "mrWidget|Merge failed." msgstr "" @@ -42637,9 +42640,6 @@ msgstr "" msgid "mrWidget|The pipeline for this merge request did not complete. Push a new commit to fix the failure, or check the %{linkStart}troubleshooting documentation%{linkEnd} to see other possible actions." msgstr "" -msgid "mrWidget|The pipeline for this merge request did not complete. Push a new commit to fix the failure." -msgstr "" - msgid "mrWidget|The source branch has been deleted" msgstr "" diff --git a/scripts/gitaly-test-build b/scripts/gitaly-test-build index adc9b56ca4f..e6afadccc7e 100755 --- a/scripts/gitaly-test-build +++ b/scripts/gitaly-test-build @@ -13,6 +13,8 @@ class GitalyTestBuild include GitalySetup def run + set_bundler_config + # If we have the binaries from the cache, we can skip building them again if File.exist?(tmp_tests_gitaly_bin_dir) GitalySetup::LOGGER.debug "Gitaly binary already built. Skip building...\n" diff --git a/scripts/gitaly-test-spawn b/scripts/gitaly-test-spawn index 3bf2de76760..e7e25a217b2 100755 --- a/scripts/gitaly-test-spawn +++ b/scripts/gitaly-test-spawn @@ -9,8 +9,17 @@ class GitalyTestSpawn include GitalySetup def run - install_gitaly_gems - spawn_gitaly + set_bundler_config + install_gitaly_gems if ENV['CI'] + check_gitaly_config! + + # # Uncomment line below to see all gitaly logs merged into CI trace + # spawn('sleep 1; tail -f log/gitaly-test.log') + + # In local development this pid file is used by rspec. + IO.write(File.expand_path('../tmp/tests/gitaly.pid', __dir__), start_gitaly) + IO.write(File.expand_path('../tmp/tests/gitaly2.pid', __dir__), start_gitaly2) + IO.write(File.expand_path('../tmp/tests/praefect.pid', __dir__), start_praefect) end end diff --git a/spec/frontend/issues_list/components/issue_card_time_info_spec.js b/spec/frontend/issues/list/components/issue_card_time_info_spec.js index 7c5faeb8dc1..e9c48b60da4 100644 --- a/spec/frontend/issues_list/components/issue_card_time_info_spec.js +++ b/spec/frontend/issues/list/components/issue_card_time_info_spec.js @@ -1,7 +1,7 @@ import { GlIcon, GlLink } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import { useFakeDate } from 'helpers/fake_date'; -import IssueCardTimeInfo from '~/issues_list/components/issue_card_time_info.vue'; +import IssueCardTimeInfo from '~/issues/list/components/issue_card_time_info.vue'; describe('CE IssueCardTimeInfo component', () => { useFakeDate(2020, 11, 11); diff --git a/spec/frontend/issues_list/components/issues_list_app_spec.js b/spec/frontend/issues/list/components/issues_list_app_spec.js index f24c090fa92..66428ee0492 100644 --- a/spec/frontend/issues_list/components/issues_list_app_spec.js +++ b/spec/frontend/issues/list/components/issues_list_app_spec.js @@ -5,8 +5,8 @@ import AxiosMockAdapter from 'axios-mock-adapter'; import { cloneDeep } from 'lodash'; import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; -import getIssuesQuery from 'ee_else_ce/issues_list/queries/get_issues.query.graphql'; -import getIssuesCountsQuery from 'ee_else_ce/issues_list/queries/get_issues_counts.query.graphql'; +import getIssuesQuery from 'ee_else_ce/issues/list/queries/get_issues.query.graphql'; +import getIssuesCountsQuery from 'ee_else_ce/issues/list/queries/get_issues_counts.query.graphql'; import createMockApollo from 'helpers/mock_apollo_helper'; import setWindowLocation from 'helpers/set_window_location_helper'; import { TEST_HOST } from 'helpers/test_constants'; @@ -17,15 +17,15 @@ import { filteredTokens, locationSearch, urlParams, -} from 'jest/issues_list/mock_data'; +} from 'jest/issues/list/mock_data'; import createFlash, { FLASH_TYPES } from '~/flash'; import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; import CsvImportExportButtons from '~/issuable/components/csv_import_export_buttons.vue'; import IssuableByEmail from '~/issuable/components/issuable_by_email.vue'; import IssuableList from '~/vue_shared/issuable/list/components/issuable_list_root.vue'; import { IssuableListTabs, IssuableStates } from '~/vue_shared/issuable/list/constants'; -import IssuesListApp from '~/issues_list/components/issues_list_app.vue'; -import NewIssueDropdown from '~/issues_list/components/new_issue_dropdown.vue'; +import IssuesListApp from '~/issues/list/components/issues_list_app.vue'; +import NewIssueDropdown from '~/issues/list/components/new_issue_dropdown.vue'; import { CREATED_DESC, DUE_DATE_OVERDUE, @@ -41,9 +41,9 @@ import { TOKEN_TYPE_RELEASE, TOKEN_TYPE_TYPE, urlSortParams, -} from '~/issues_list/constants'; -import eventHub from '~/issues_list/eventhub'; -import { getSortOptions } from '~/issues_list/utils'; +} from '~/issues/list/constants'; +import eventHub from '~/issues/list/eventhub'; +import { getSortOptions } from '~/issues/list/utils'; import axios from '~/lib/utils/axios_utils'; import { scrollUp } from '~/lib/utils/scroll_utils'; import { joinPaths } from '~/lib/utils/url_utility'; diff --git a/spec/frontend/issues_list/components/jira_issues_import_status_app_spec.js b/spec/frontend/issues/list/components/jira_issues_import_status_app_spec.js index 633799816d8..d6d6bb14e9d 100644 --- a/spec/frontend/issues_list/components/jira_issues_import_status_app_spec.js +++ b/spec/frontend/issues/list/components/jira_issues_import_status_app_spec.js @@ -1,7 +1,7 @@ import { GlAlert, GlLabel } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import Vue from 'vue'; -import JiraIssuesImportStatus from '~/issues_list/components/jira_issues_import_status_app.vue'; +import JiraIssuesImportStatus from '~/issues/list/components/jira_issues_import_status_app.vue'; describe('JiraIssuesImportStatus', () => { const issuesPath = 'gitlab-org/gitlab-test/-/issues'; diff --git a/spec/frontend/issues_list/components/new_issue_dropdown_spec.js b/spec/frontend/issues/list/components/new_issue_dropdown_spec.js index 1c9a87e8af2..0c52e66ff14 100644 --- a/spec/frontend/issues_list/components/new_issue_dropdown_spec.js +++ b/spec/frontend/issues/list/components/new_issue_dropdown_spec.js @@ -2,8 +2,8 @@ import { GlDropdown, GlDropdownItem, GlSearchBoxByType } from '@gitlab/ui'; import { createLocalVue, mount, shallowMount } from '@vue/test-utils'; import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; -import NewIssueDropdown from '~/issues_list/components/new_issue_dropdown.vue'; -import searchProjectsQuery from '~/issues_list/queries/search_projects.query.graphql'; +import NewIssueDropdown from '~/issues/list/components/new_issue_dropdown.vue'; +import searchProjectsQuery from '~/issues/list/queries/search_projects.query.graphql'; import { DASH_SCOPE, joinPaths } from '~/lib/utils/url_utility'; import { emptySearchProjectsQueryResponse, diff --git a/spec/frontend/issues_list/mock_data.js b/spec/frontend/issues/list/mock_data.js index 948699876ce..948699876ce 100644 --- a/spec/frontend/issues_list/mock_data.js +++ b/spec/frontend/issues/list/mock_data.js diff --git a/spec/frontend/issues_list/utils_spec.js b/spec/frontend/issues/list/utils_spec.js index 8e1d70db92d..0e4979fd7b4 100644 --- a/spec/frontend/issues_list/utils_spec.js +++ b/spec/frontend/issues/list/utils_spec.js @@ -7,14 +7,14 @@ import { locationSearchWithSpecialValues, urlParams, urlParamsWithSpecialValues, -} from 'jest/issues_list/mock_data'; +} from 'jest/issues/list/mock_data'; import { defaultPageSizeParams, DUE_DATE_VALUES, largePageSizeParams, RELATIVE_POSITION_ASC, urlSortParams, -} from '~/issues_list/constants'; +} from '~/issues/list/constants'; import { convertToApiParams, convertToSearchQuery, @@ -24,7 +24,7 @@ import { getInitialPageParams, getSortKey, getSortOptions, -} from '~/issues_list/utils'; +} from '~/issues/list/utils'; describe('getInitialPageParams', () => { it.each(Object.keys(urlSortParams))( diff --git a/spec/frontend/vue_mr_widget/components/states/merge_checks_failed_spec.js b/spec/frontend/vue_mr_widget/components/states/merge_checks_failed_spec.js index bdad0bada5f..1900b53ac11 100644 --- a/spec/frontend/vue_mr_widget/components/states/merge_checks_failed_spec.js +++ b/spec/frontend/vue_mr_widget/components/states/merge_checks_failed_spec.js @@ -15,35 +15,12 @@ describe('Merge request widget merge checks failed state component', () => { }); it.each` - mrState | displayText - ${{ isPipelineFailed: true }} | ${'pipelineFailed'} - ${{ approvals: true, isApproved: false }} | ${'approvalNeeded'} - ${{ hasMergeableDiscussionsState: true }} | ${'unresolvedDiscussions'} + mrState | displayText + ${{ approvals: true, isApproved: false }} | ${'approvalNeeded'} + ${{ blockingMergeRequests: { total_count: 1 } }} | ${'blockingMergeRequests'} `('display $displayText text for $mrState', ({ mrState, displayText }) => { factory({ mr: mrState }); expect(wrapper.text()).toContain(MergeChecksFailed.i18n[displayText]); }); - - describe('unresolved discussions', () => { - it('renders jump to button', () => { - factory({ mr: { hasMergeableDiscussionsState: true } }); - - expect(wrapper.find('[data-testid="jumpToUnresolved"]').exists()).toBe(true); - }); - - it('renders resolve thread button', () => { - factory({ - mr: { - hasMergeableDiscussionsState: true, - createIssueToResolveDiscussionsPath: 'https://gitlab.com', - }, - }); - - expect(wrapper.find('[data-testid="resolveIssue"]').exists()).toBe(true); - expect(wrapper.find('[data-testid="resolveIssue"]').attributes('href')).toBe( - 'https://gitlab.com', - ); - }); - }); }); diff --git a/spec/lib/backup/gitaly_backup_spec.rb b/spec/lib/backup/gitaly_backup_spec.rb index 2ccde517533..cd0d984fbdb 100644 --- a/spec/lib/backup/gitaly_backup_spec.rb +++ b/spec/lib/backup/gitaly_backup_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' RSpec.describe Backup::GitalyBackup do - let(:parallel) { nil } - let(:parallel_storage) { nil } + let(:max_parallelism) { nil } + let(:storage_parallelism) { nil } let(:progress) do Tempfile.new('progress').tap do |progress| @@ -23,7 +23,7 @@ RSpec.describe Backup::GitalyBackup do progress.close end - subject { described_class.new(progress, parallel: parallel, parallel_storage: parallel_storage) } + subject { described_class.new(progress, max_parallelism: max_parallelism, storage_parallelism: storage_parallelism) } context 'unknown' do it 'fails to start unknown' do @@ -48,7 +48,7 @@ RSpec.describe Backup::GitalyBackup do subject.enqueue(project, Gitlab::GlRepository::DESIGN) subject.enqueue(personal_snippet, Gitlab::GlRepository::SNIPPET) subject.enqueue(project_snippet, Gitlab::GlRepository::SNIPPET) - subject.wait + subject.finish! expect(File).to exist(File.join(Gitlab.config.backup.path, 'repositories', project.disk_path + '.bundle')) expect(File).to exist(File.join(Gitlab.config.backup.path, 'repositories', project.disk_path + '.wiki.bundle')) @@ -58,24 +58,24 @@ RSpec.describe Backup::GitalyBackup do end context 'parallel option set' do - let(:parallel) { 3 } + let(:max_parallelism) { 3 } it 'passes parallel option through' do expect(Open3).to receive(:popen2).with(expected_env, anything, 'create', '-path', anything, '-parallel', '3').and_call_original subject.start(:create) - subject.wait + subject.finish! end end context 'parallel_storage option set' do - let(:parallel_storage) { 3 } + let(:storage_parallelism) { 3 } it 'passes parallel option through' do expect(Open3).to receive(:popen2).with(expected_env, anything, 'create', '-path', anything, '-parallel-storage', '3').and_call_original subject.start(:create) - subject.wait + subject.finish! end end @@ -83,7 +83,7 @@ RSpec.describe Backup::GitalyBackup do expect(subject).to receive(:bin_path).and_return(Gitlab::Utils.which('false')) subject.start(:create) - expect { subject.wait }.to raise_error(::Backup::Error, 'gitaly-backup exit status 1') + expect { subject.finish! }.to raise_error(::Backup::Error, 'gitaly-backup exit status 1') end end @@ -115,7 +115,7 @@ RSpec.describe Backup::GitalyBackup do expect(Open3).to receive(:popen2).with(ssl_env, anything, 'create', '-path', anything).and_call_original subject.start(:create) - subject.wait + subject.finish! end end end @@ -145,7 +145,7 @@ RSpec.describe Backup::GitalyBackup do subject.enqueue(project, Gitlab::GlRepository::DESIGN) subject.enqueue(personal_snippet, Gitlab::GlRepository::SNIPPET) subject.enqueue(project_snippet, Gitlab::GlRepository::SNIPPET) - subject.wait + subject.finish! collect_commit_shas = -> (repo) { repo.commits('master', limit: 10).map(&:sha) } @@ -157,24 +157,24 @@ RSpec.describe Backup::GitalyBackup do end context 'parallel option set' do - let(:parallel) { 3 } + let(:max_parallelism) { 3 } it 'passes parallel option through' do expect(Open3).to receive(:popen2).with(expected_env, anything, 'restore', '-path', anything, '-parallel', '3').and_call_original subject.start(:restore) - subject.wait + subject.finish! end end context 'parallel_storage option set' do - let(:parallel_storage) { 3 } + let(:storage_parallelism) { 3 } it 'passes parallel option through' do expect(Open3).to receive(:popen2).with(expected_env, anything, 'restore', '-path', anything, '-parallel-storage', '3').and_call_original subject.start(:restore) - subject.wait + subject.finish! end end @@ -182,7 +182,7 @@ RSpec.describe Backup::GitalyBackup do expect(subject).to receive(:bin_path).and_return(Gitlab::Utils.which('false')) subject.start(:restore) - expect { subject.wait }.to raise_error(::Backup::Error, 'gitaly-backup exit status 1') + expect { subject.finish! }.to raise_error(::Backup::Error, 'gitaly-backup exit status 1') end end end diff --git a/spec/lib/backup/gitaly_rpc_backup_spec.rb b/spec/lib/backup/gitaly_rpc_backup_spec.rb index fb442f4a86f..14f9d27ca6e 100644 --- a/spec/lib/backup/gitaly_rpc_backup_spec.rb +++ b/spec/lib/backup/gitaly_rpc_backup_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Backup::GitalyRpcBackup do subject.enqueue(project, Gitlab::GlRepository::DESIGN) subject.enqueue(personal_snippet, Gitlab::GlRepository::SNIPPET) subject.enqueue(project_snippet, Gitlab::GlRepository::SNIPPET) - subject.wait + subject.finish! expect(File).to exist(File.join(Gitlab.config.backup.path, 'repositories', project.disk_path + '.bundle')) expect(File).to exist(File.join(Gitlab.config.backup.path, 'repositories', project.disk_path + '.wiki.bundle')) @@ -52,7 +52,7 @@ RSpec.describe Backup::GitalyRpcBackup do it 'logs an appropriate message', :aggregate_failures do subject.start(:create) subject.enqueue(project, Gitlab::GlRepository::PROJECT) - subject.wait + subject.finish! expect(progress).to have_received(:puts).with("[Failed] backing up #{project.full_path} (#{project.disk_path})") expect(progress).to have_received(:puts).with("Error Fail in tests") @@ -96,7 +96,7 @@ RSpec.describe Backup::GitalyRpcBackup do subject.enqueue(project, Gitlab::GlRepository::DESIGN) subject.enqueue(personal_snippet, Gitlab::GlRepository::SNIPPET) subject.enqueue(project_snippet, Gitlab::GlRepository::SNIPPET) - subject.wait + subject.finish! collect_commit_shas = -> (repo) { repo.commits('master', limit: 10).map(&:sha) } @@ -129,7 +129,7 @@ RSpec.describe Backup::GitalyRpcBackup do subject.enqueue(project, Gitlab::GlRepository::DESIGN) subject.enqueue(personal_snippet, Gitlab::GlRepository::SNIPPET) subject.enqueue(project_snippet, Gitlab::GlRepository::SNIPPET) - subject.wait + subject.finish! end context 'failure' do @@ -143,7 +143,7 @@ RSpec.describe Backup::GitalyRpcBackup do it 'logs an appropriate message', :aggregate_failures do subject.start(:restore) subject.enqueue(project, Gitlab::GlRepository::PROJECT) - subject.wait + subject.finish! expect(progress).to have_received(:puts).with("[Failed] restoring #{project.full_path} (#{project.disk_path})") expect(progress).to have_received(:puts).with("Error Fail in tests") diff --git a/spec/lib/backup/repositories_spec.rb b/spec/lib/backup/repositories_spec.rb index 85818038c9d..f3830da344b 100644 --- a/spec/lib/backup/repositories_spec.rb +++ b/spec/lib/backup/repositories_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Backup::Repositories do expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::DESIGN) expect(strategy).to have_received(:enqueue).with(project_snippet, Gitlab::GlRepository::SNIPPET) expect(strategy).to have_received(:enqueue).with(personal_snippet, Gitlab::GlRepository::SNIPPET) - expect(strategy).to have_received(:wait) + expect(strategy).to have_received(:finish!) end end @@ -49,7 +49,7 @@ RSpec.describe Backup::Repositories do projects.each do |project| expect(strategy).to receive(:enqueue).with(project, Gitlab::GlRepository::PROJECT) end - expect(strategy).to receive(:wait) + expect(strategy).to receive(:finish!) subject.dump(max_concurrency: 1, max_storage_concurrency: 1) end @@ -91,7 +91,7 @@ RSpec.describe Backup::Repositories do projects.each do |project| expect(strategy).to receive(:enqueue).with(project, Gitlab::GlRepository::PROJECT) end - expect(strategy).to receive(:wait) + expect(strategy).to receive(:finish!) subject.dump(max_concurrency: 2, max_storage_concurrency: 2) end @@ -114,7 +114,7 @@ RSpec.describe Backup::Repositories do projects.each do |project| expect(strategy).to receive(:enqueue).with(project, Gitlab::GlRepository::PROJECT) end - expect(strategy).to receive(:wait) + expect(strategy).to receive(:finish!) subject.dump(max_concurrency: 1, max_storage_concurrency: max_storage_concurrency) end @@ -128,7 +128,7 @@ RSpec.describe Backup::Repositories do projects.each do |project| expect(strategy).to receive(:enqueue).with(project, Gitlab::GlRepository::PROJECT) end - expect(strategy).to receive(:wait) + expect(strategy).to receive(:finish!) subject.dump(max_concurrency: 3, max_storage_concurrency: max_storage_concurrency) end @@ -184,7 +184,7 @@ RSpec.describe Backup::Repositories do expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::DESIGN) expect(strategy).to have_received(:enqueue).with(project_snippet, Gitlab::GlRepository::SNIPPET) expect(strategy).to have_received(:enqueue).with(personal_snippet, Gitlab::GlRepository::SNIPPET) - expect(strategy).to have_received(:wait) + expect(strategy).to have_received(:finish!) end context 'restoring object pools' do diff --git a/spec/lib/gitlab/middleware/webhook_recursion_detection_spec.rb b/spec/lib/gitlab/middleware/webhook_recursion_detection_spec.rb new file mode 100644 index 00000000000..c8dbc990f8c --- /dev/null +++ b/spec/lib/gitlab/middleware/webhook_recursion_detection_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'action_dispatch' +require 'rack' +require 'request_store' + +RSpec.describe Gitlab::Middleware::WebhookRecursionDetection do + let(:app) { double(:app) } + let(:middleware) { described_class.new(app) } + let(:env) { Rack::MockRequest.env_for("/").merge(headers) } + + around do |example| + Gitlab::WithRequestStore.with_request_store { example.run } + end + + describe '#call' do + subject(:call) { described_class.new(app).call(env) } + + context 'when the recursion detection header is present' do + let(:new_uuid) { SecureRandom.uuid } + let(:headers) { { 'HTTP_X_GITLAB_EVENT_UUID' => new_uuid } } + + it 'sets the request UUID from the header' do + expect(app).to receive(:call) + expect { call }.to change { Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid }.to(new_uuid) + end + end + + context 'when recursion headers are not present' do + let(:headers) { {} } + + it 'works without errors' do + expect(app).to receive(:call) + + call + + expect(Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid).to be_nil + end + end + end +end diff --git a/spec/lib/gitlab/web_hooks/recursion_detection_spec.rb b/spec/lib/gitlab/web_hooks/recursion_detection_spec.rb new file mode 100644 index 00000000000..4f92aa8b3d1 --- /dev/null +++ b/spec/lib/gitlab/web_hooks/recursion_detection_spec.rb @@ -0,0 +1,243 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::WebHooks::RecursionDetection, :clean_gitlab_redis_shared_state, :request_store do + let_it_be(:web_hook) { create(:project_hook) } + + let!(:uuid_class) { described_class::UUID } + + describe '.set_from_headers' do + let(:old_uuid) { SecureRandom.uuid } + let(:rack_headers) { Rack::MockRequest.env_for("/").merge(headers) } + + subject(:set_from_headers) { described_class.set_from_headers(rack_headers) } + + # Note, having a previous `request_uuid` value set before `.set_from_headers` is + # called is contrived and should not normally happen. However, testing with this scenario + # allows us to assert the ideal outcome if it ever were to happen. + before do + uuid_class.instance.request_uuid = old_uuid + end + + context 'when the detection header is present' do + let(:new_uuid) { SecureRandom.uuid } + + let(:headers) do + { uuid_class::HEADER => new_uuid } + end + + it 'sets the request UUID value from the headers' do + set_from_headers + + expect(uuid_class.instance.request_uuid).to eq(new_uuid) + end + end + + context 'when detection header is not present' do + let(:headers) { {} } + + it 'does not set the request UUID' do + set_from_headers + + expect(uuid_class.instance.request_uuid).to eq(old_uuid) + end + end + end + + describe '.set_request_uuid' do + it 'sets the request UUID value' do + new_uuid = SecureRandom.uuid + + described_class.set_request_uuid(new_uuid) + + expect(uuid_class.instance.request_uuid).to eq(new_uuid) + end + end + + describe '.register!' do + let_it_be(:second_web_hook) { create(:project_hook) } + let_it_be(:third_web_hook) { create(:project_hook) } + + def cache_key(hook) + described_class.send(:cache_key_for_hook, hook) + end + + it 'stores IDs in the same cache when a request UUID is set, until the request UUID changes', :aggregate_failures do + # Register web_hook and second_web_hook against the same request UUID. + uuid_class.instance.request_uuid = SecureRandom.uuid + described_class.register!(web_hook) + described_class.register!(second_web_hook) + first_cache_key = cache_key(web_hook) + second_cache_key = cache_key(second_web_hook) + + # Register third_web_hook against a new request UUID. + uuid_class.instance.request_uuid = SecureRandom.uuid + described_class.register!(third_web_hook) + third_cache_key = cache_key(third_web_hook) + + expect(first_cache_key).to eq(second_cache_key) + expect(second_cache_key).not_to eq(third_cache_key) + + ::Gitlab::Redis::SharedState.with do |redis| + members = redis.smembers(first_cache_key).map(&:to_i) + expect(members).to contain_exactly(web_hook.id, second_web_hook.id) + + members = redis.smembers(third_cache_key).map(&:to_i) + expect(members).to contain_exactly(third_web_hook.id) + end + end + + it 'stores IDs in unique caches when no request UUID is present', :aggregate_failures do + described_class.register!(web_hook) + described_class.register!(second_web_hook) + described_class.register!(third_web_hook) + + first_cache_key = cache_key(web_hook) + second_cache_key = cache_key(second_web_hook) + third_cache_key = cache_key(third_web_hook) + + expect([first_cache_key, second_cache_key, third_cache_key].compact.length).to eq(3) + + ::Gitlab::Redis::SharedState.with do |redis| + members = redis.smembers(first_cache_key).map(&:to_i) + expect(members).to contain_exactly(web_hook.id) + + members = redis.smembers(second_cache_key).map(&:to_i) + expect(members).to contain_exactly(second_web_hook.id) + + members = redis.smembers(third_cache_key).map(&:to_i) + expect(members).to contain_exactly(third_web_hook.id) + end + end + + it 'touches the storage ttl each time it is called', :aggregate_failures do + freeze_time do + described_class.register!(web_hook) + + ::Gitlab::Redis::SharedState.with do |redis| + expect(redis.ttl(cache_key(web_hook))).to eq(described_class::TOUCH_CACHE_TTL.to_i) + end + end + + travel_to(1.minute.from_now) do + described_class.register!(second_web_hook) + + ::Gitlab::Redis::SharedState.with do |redis| + expect(redis.ttl(cache_key(web_hook))).to eq(described_class::TOUCH_CACHE_TTL.to_i) + end + end + end + + it 'does not store anything if the feature flag is disabled' do + stub_feature_flags(webhook_recursion_detection: false) + + described_class.register!(web_hook) + + ::Gitlab::Redis::SharedState.with do |redis| + expect(redis.exists(cache_key(web_hook))).to eq(false) + end + end + end + + describe 'block?' do + let_it_be(:registered_web_hooks) { create_list(:project_hook, 2) } + + subject(:block?) { described_class.block?(web_hook) } + + before do + # Register some previous webhooks. + uuid_class.instance.request_uuid = SecureRandom.uuid + + registered_web_hooks.each do |web_hook| + described_class.register!(web_hook) + end + end + + it 'returns false if webhook should not be blocked' do + is_expected.to eq(false) + end + + context 'when the webhook has previously fired' do + before do + described_class.register!(web_hook) + end + + it 'returns true' do + is_expected.to eq(true) + end + + it 'returns false if the feature flag is disabled' do + stub_feature_flags(webhook_recursion_detection: false) + + is_expected.to eq(false) + end + + context 'when the request UUID changes again' do + before do + uuid_class.instance.request_uuid = SecureRandom.uuid + end + + it 'returns false' do + is_expected.to eq(false) + end + end + end + + context 'when the count limit has been reached' do + let_it_be(:registered_web_hooks) { create_list(:project_hook, 2) } + + before do + registered_web_hooks.each do |web_hook| + described_class.register!(web_hook) + end + + stub_const("#{described_class.name}::COUNT_LIMIT", registered_web_hooks.size) + end + + it 'returns true' do + is_expected.to eq(true) + end + + it 'returns false if the feature flag is disabled' do + stub_feature_flags(webhook_recursion_detection: false) + + is_expected.to eq(false) + end + + context 'when the request UUID changes again' do + before do + uuid_class.instance.request_uuid = SecureRandom.uuid + end + + it 'returns false' do + is_expected.to eq(false) + end + end + end + end + + describe '.header' do + subject(:header) { described_class.header(web_hook) } + + it 'returns a header with the UUID value' do + uuid = SecureRandom.uuid + allow(uuid_class.instance).to receive(:uuid_for_hook).and_return(uuid) + + is_expected.to eq({ uuid_class::HEADER => uuid }) + end + end + + describe '.to_log' do + subject(:to_log) { described_class.to_log(web_hook) } + + it 'returns the UUID value and all registered webhook IDs in a Hash' do + uuid = SecureRandom.uuid + allow(uuid_class.instance).to receive(:uuid_for_hook).and_return(uuid) + registered_web_hooks = create_list(:project_hook, 2) + registered_web_hooks.each { described_class.register!(_1) } + + is_expected.to eq({ uuid: uuid, ids: registered_web_hooks.map(&:id) }) + end + end +end diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index f0ee9a613d8..ec2eca96755 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -40,6 +40,15 @@ RSpec.describe ProjectHook do end end + describe '#parent' do + it 'returns the associated project' do + project = build(:project) + hook = build(:project_hook, project: project) + + expect(hook.parent).to eq(project) + end + end + describe '#application_context' do let_it_be(:hook) { build(:project_hook) } diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 4ce2e729d89..85f433f5f81 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -31,6 +31,36 @@ RSpec.describe ServiceHook do end end + describe '#parent' do + let(:hook) { build(:service_hook, integration: integration) } + + context 'with a project-level integration' do + let(:project) { build(:project) } + let(:integration) { build(:integration, project: project) } + + it 'returns the associated project' do + expect(hook.parent).to eq(project) + end + end + + context 'with a group-level integration' do + let(:group) { build(:group) } + let(:integration) { build(:integration, :group, group: group) } + + it 'returns the associated group' do + expect(hook.parent).to eq(group) + end + end + + context 'with an instance-level integration' do + let(:integration) { build(:integration, :instance) } + + it 'returns nil' do + expect(hook.parent).to be_nil + end + end + end + describe '#application_context' do let(:hook) { build(:service_hook) } diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb index 9c3ff7aa35b..0607505e7d5 100644 --- a/spec/models/integrations/datadog_spec.rb +++ b/spec/models/integrations/datadog_spec.rb @@ -167,7 +167,7 @@ RSpec.describe Integrations::Datadog do context 'with pipeline data' do let(:data) { pipeline_data } - let(:expected_headers) { { WebHookService::GITLAB_EVENT_HEADER => 'Pipeline Hook' } } + let(:expected_headers) { { ::Gitlab::WebHooks::GITLAB_EVENT_HEADER => 'Pipeline Hook' } } let(:expected_body) { data.with_retried_builds.to_json } it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made } @@ -175,7 +175,7 @@ RSpec.describe Integrations::Datadog do context 'with job data' do let(:data) { build_data } - let(:expected_headers) { { WebHookService::GITLAB_EVENT_HEADER => 'Job Hook' } } + let(:expected_headers) { { ::Gitlab::WebHooks::GITLAB_EVENT_HEADER => 'Job Hook' } } let(:expected_body) { data.to_json } it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made } diff --git a/spec/requests/api/ci/triggers_spec.rb b/spec/requests/api/ci/triggers_spec.rb index d270a16d28d..a036a55f5f3 100644 --- a/spec/requests/api/ci/triggers_spec.rb +++ b/spec/requests/api/ci/triggers_spec.rb @@ -162,7 +162,7 @@ RSpec.describe API::Ci::Triggers do expect do post api("/projects/#{project.id}/ref/master/trigger/pipeline?token=#{trigger_token}"), params: { ref: 'refs/heads/other-branch' }, - headers: { WebHookService::GITLAB_EVENT_HEADER => 'Pipeline Hook' } + headers: { ::Gitlab::WebHooks::GITLAB_EVENT_HEADER => 'Pipeline Hook' } end.not_to change(Ci::Pipeline, :count) expect(response).to have_gitlab_http_status(:forbidden) diff --git a/spec/requests/recursive_webhook_detection_spec.rb b/spec/requests/recursive_webhook_detection_spec.rb new file mode 100644 index 00000000000..09a352da84b --- /dev/null +++ b/spec/requests/recursive_webhook_detection_spec.rb @@ -0,0 +1,200 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_redis_shared_state, :request_store do + include StubRequests + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository, namespace: user.namespace, creator: user) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + let_it_be(:project_hook) { create(:project_hook, project: project, merge_requests_events: true) } + let_it_be(:system_hook) { create(:system_hook, merge_requests_events: true) } + + # Trigger a change to the merge request to fire the webhooks. + def trigger_web_hooks + params = { merge_request: { description: FFaker::Lorem.sentence } } + put project_merge_request_path(project, merge_request), params: params, headers: headers + end + + def stub_requests + stub_full_request(project_hook.url, method: :post, ip_address: '8.8.8.8') + stub_full_request(system_hook.url, method: :post, ip_address: '8.8.8.9') + end + + before do + login_as(user) + end + + context 'when the request headers include the recursive webhook detection header' do + let(:uuid) { SecureRandom.uuid } + let(:headers) { { Gitlab::WebHooks::RecursionDetection::UUID::HEADER => uuid } } + + it 'executes all webhooks, logs no errors, and the webhook requests contain the same UUID header', :aggregate_failures do + stub_requests + + expect(Gitlab::AuthLogger).not_to receive(:error) + + trigger_web_hooks + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with { |req| req.headers['X-Gitlab-Event-Uuid'] == uuid } + .once + expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)) + .with { |req| req.headers['X-Gitlab-Event-Uuid'] == uuid } + .once + end + + shared_examples 'when the feature flag is disabled' do + it 'executes and logs no errors' do + stub_feature_flags(webhook_recursion_detection: false) + stub_requests + + expect(Gitlab::AuthLogger).not_to receive(:error) + + trigger_web_hooks + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)).once + expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).once + end + end + + context 'when one of the webhooks is recursive' do + before do + # Recreate the necessary state for the previous request to be + # considered made from the webhook. + Gitlab::WebHooks::RecursionDetection.set_request_uuid(uuid) + Gitlab::WebHooks::RecursionDetection.register!(project_hook) + Gitlab::WebHooks::RecursionDetection.set_request_uuid(nil) + end + + it 'executes all webhooks and logs an error for the recursive hook', :aggregate_failures do + stub_requests + + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Webhook recursion detected and will be blocked in future', + hook_id: project_hook.id, + recursion_detection: { + uuid: uuid, + ids: [project_hook.id] + } + ) + ).twice # Twice: once in `#async_execute`, and again in `#execute`. + + trigger_web_hooks + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)).once + expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).once + end + + include_examples 'when the feature flag is disabled' + end + + context 'when the count limit has been reached' do + let_it_be(:previous_hooks) { create_list(:project_hook, 3) } + + before do + stub_const('Gitlab::WebHooks::RecursionDetection::COUNT_LIMIT', 2) + # Recreate the necessary state for a number of previous webhooks to + # have been triggered previously. + Gitlab::WebHooks::RecursionDetection.set_request_uuid(uuid) + previous_hooks.each { Gitlab::WebHooks::RecursionDetection.register!(_1) } + Gitlab::WebHooks::RecursionDetection.set_request_uuid(nil) + end + + it 'executes and logs errors for all hooks', :aggregate_failures do + stub_requests + previous_hook_ids = previous_hooks.map(&:id) + + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Webhook recursion detected and will be blocked in future', + hook_id: project_hook.id, + recursion_detection: { + uuid: uuid, + ids: include(*previous_hook_ids) + } + ) + ).twice + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Webhook recursion detected and will be blocked in future', + hook_id: system_hook.id, + recursion_detection: { + uuid: uuid, + ids: include(*previous_hook_ids) + } + ) + ).twice + + trigger_web_hooks + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)).once + expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).once + end + end + + include_examples 'when the feature flag is disabled' + end + + context 'when the recursive webhook detection header is absent' do + let(:headers) { {} } + + let(:uuid_header_spy) do + Class.new do + attr_reader :values + + def initialize + @values = [] + end + + def to_proc + proc do |method, *args| + method.call(*args).tap do |headers| + @values << headers[Gitlab::WebHooks::RecursionDetection::UUID::HEADER] + end + end + end + end.new + end + + before do + allow(Gitlab::WebHooks::RecursionDetection).to receive(:header).at_least(:once).and_wrap_original(&uuid_header_spy) + end + + it 'executes all webhooks, logs no errors, and the webhook requests contain different UUID headers', :aggregate_failures do + stub_requests + + expect(Gitlab::AuthLogger).not_to receive(:error) + + trigger_web_hooks + + uuid_headers = uuid_header_spy.values + + expect(uuid_headers).to all(be_present) + expect(uuid_headers.uniq.length).to eq(2) + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with { |req| uuid_headers.include?(req.headers['X-Gitlab-Event-Uuid']) } + .once + expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)) + .with { |req| uuid_headers.include?(req.headers['X-Gitlab-Event-Uuid']) } + .once + end + + it 'uses new UUID values between requests' do + stub_requests + + trigger_web_hooks + trigger_web_hooks + + uuid_headers = uuid_header_spy.values + + expect(uuid_headers).to all(be_present) + expect(uuid_headers.length).to eq(4) + expect(uuid_headers.uniq.length).to eq(4) + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)).twice + expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).twice + end + end +end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 2aebd2adab9..7d933ea9c5c 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -2,20 +2,12 @@ require 'spec_helper' -RSpec.describe WebHookService do +RSpec.describe WebHookService, :request_store, :clean_gitlab_redis_shared_state do include StubRequests let_it_be(:project) { create(:project) } let_it_be_with_reload(:project_hook) { create(:project_hook, project: project) } - let(:headers) do - { - 'Content-Type' => 'application/json', - 'User-Agent' => "GitLab/#{Gitlab::VERSION}", - 'X-Gitlab-Event' => 'Push Hook' - } - end - let(:data) do { before: 'oldrev', after: 'newrev', ref: 'ref' } end @@ -61,6 +53,21 @@ RSpec.describe WebHookService do end describe '#execute' do + let!(:uuid) { SecureRandom.uuid } + let(:headers) do + { + 'Content-Type' => 'application/json', + 'User-Agent' => "GitLab/#{Gitlab::VERSION}", + 'X-Gitlab-Event' => 'Push Hook', + 'X-Gitlab-Event-UUID' => uuid + } + end + + before do + # Set a stable value for the `X-Gitlab-Event-UUID` header. + Gitlab::WebHooks::RecursionDetection.set_request_uuid(uuid) + end + context 'when token is defined' do let_it_be(:project_hook) { create(:project_hook, :token) } @@ -127,11 +134,74 @@ RSpec.describe WebHookService do expect(service_instance.execute).to eq({ status: :error, message: 'Hook disabled' }) end + it 'executes and registers the hook with the recursion detection', :aggregate_failures do + stub_full_request(project_hook.url, method: :post) + cache_key = Gitlab::WebHooks::RecursionDetection.send(:cache_key_for_hook, project_hook) + + ::Gitlab::Redis::SharedState.with do |redis| + expect { service_instance.execute }.to change { + redis.sismember(cache_key, project_hook.id) + }.to(true) + end + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: headers) + .once + end + + it 'executes and logs if a recursive web hook is detected', :aggregate_failures do + stub_full_request(project_hook.url, method: :post) + Gitlab::WebHooks::RecursionDetection.register!(project_hook) + + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Webhook recursion detected and will be blocked in future', + hook_id: project_hook.id, + hook_type: 'ProjectHook', + hook_name: 'push_hooks', + recursion_detection: Gitlab::WebHooks::RecursionDetection.to_log(project_hook), + 'correlation_id' => kind_of(String) + ) + ) + + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: headers) + .once + end + + it 'executes and logs if the recursion count limit would be exceeded', :aggregate_failures do + stub_full_request(project_hook.url, method: :post) + stub_const("#{Gitlab::WebHooks::RecursionDetection.name}::COUNT_LIMIT", 3) + previous_hooks = create_list(:project_hook, 3) + previous_hooks.each { Gitlab::WebHooks::RecursionDetection.register!(_1) } + + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Webhook recursion detected and will be blocked in future', + hook_id: project_hook.id, + hook_type: 'ProjectHook', + hook_name: 'push_hooks', + recursion_detection: Gitlab::WebHooks::RecursionDetection.to_log(project_hook), + 'correlation_id' => kind_of(String) + ) + ) + + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: headers) + .once + end + it 'handles exceptions' do exceptions = Gitlab::HTTP::HTTP_ERRORS + [ Gitlab::Json::LimitedEncoder::LimitExceeded, URI::InvalidURIError ] + allow(Gitlab::WebHooks::RecursionDetection).to receive(:block?).and_return(false) + exceptions.each do |exception_class| exception = exception_class.new('Exception message') project_hook.enable! @@ -420,6 +490,57 @@ RSpec.describe WebHookService do end end + context 'recursion detection' do + before do + # Set a request UUID so `RecursionDetection.block?` will query redis. + Gitlab::WebHooks::RecursionDetection.set_request_uuid(SecureRandom.uuid) + end + + it 'queues a worker and logs an error if the call chain limit would be exceeded' do + stub_const("#{Gitlab::WebHooks::RecursionDetection.name}::COUNT_LIMIT", 3) + previous_hooks = create_list(:project_hook, 3) + previous_hooks.each { Gitlab::WebHooks::RecursionDetection.register!(_1) } + + expect(WebHookWorker).to receive(:perform_async) + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Webhook recursion detected and will be blocked in future', + hook_id: project_hook.id, + hook_type: 'ProjectHook', + hook_name: 'push_hooks', + recursion_detection: Gitlab::WebHooks::RecursionDetection.to_log(project_hook), + 'correlation_id' => kind_of(String), + 'meta.project' => project.full_path, + 'meta.related_class' => 'ProjectHook', + 'meta.root_namespace' => project.root_namespace.full_path + ) + ) + + service_instance.async_execute + end + + it 'queues a worker and logs an error if a recursive call chain is detected' do + Gitlab::WebHooks::RecursionDetection.register!(project_hook) + + expect(WebHookWorker).to receive(:perform_async) + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Webhook recursion detected and will be blocked in future', + hook_id: project_hook.id, + hook_type: 'ProjectHook', + hook_name: 'push_hooks', + recursion_detection: Gitlab::WebHooks::RecursionDetection.to_log(project_hook), + 'correlation_id' => kind_of(String), + 'meta.project' => project.full_path, + 'meta.related_class' => 'ProjectHook', + 'meta.root_namespace' => project.root_namespace.full_path + ) + ) + + service_instance.async_execute + end + end + context 'when hook has custom context attributes' do it 'includes the attributes in the worker context' do expect(WebHookWorker).to receive(:perform_async) do diff --git a/spec/support/helpers/gitaly_setup.rb b/spec/support/helpers/gitaly_setup.rb index cc0ddfdedac..923051a2e04 100644 --- a/spec/support/helpers/gitaly_setup.rb +++ b/spec/support/helpers/gitaly_setup.rb @@ -9,13 +9,8 @@ require 'securerandom' require 'socket' require 'logger' -require 'bundler' module GitalySetup - extend self - - REPOS_STORAGE = 'default' - LOGGER = begin default_name = ENV['CI'] ? 'DEBUG' : 'WARN' level_name = ENV['GITLAB_TESTING_LOG_LEVEL']&.upcase @@ -59,12 +54,9 @@ module GitalySetup { 'HOME' => expand_path('tmp/tests'), 'GEM_PATH' => Gem.path.join(':'), + 'BUNDLE_APP_CONFIG' => File.join(gemfile_dir, '.bundle'), 'BUNDLE_INSTALL_FLAGS' => nil, - 'BUNDLE_IGNORE_CONFIG' => '1', - 'BUNDLE_PATH' => bundle_path, 'BUNDLE_GEMFILE' => gemfile, - 'BUNDLE_JOBS' => '4', - 'BUNDLE_RETRY' => '3', 'RUBYOPT' => nil, # Git hooks can't run during tests as the internal API is not running. @@ -73,20 +65,17 @@ module GitalySetup } end - def bundle_path - # Allow the user to override BUNDLE_PATH if they need to - return ENV['GITALY_TEST_BUNDLE_PATH'] if ENV['GITALY_TEST_BUNDLE_PATH'] + # rubocop:disable GitlabSecurity/SystemCommandInjection + def set_bundler_config + system('bundle config set --local jobs 4', chdir: gemfile_dir) + system('bundle config set --local retry 3', chdir: gemfile_dir) if ENV['CI'] - expand_path('vendor/gitaly-ruby') - else - explicit_path = Bundler.configured_bundle_path.explicit_path - - return unless explicit_path - - expand_path(explicit_path) + bundle_path = expand_path('vendor/gitaly-ruby') + system('bundle', 'config', 'set', '--local', 'path', bundle_path, chdir: gemfile_dir) end end + # rubocop:enable GitlabSecurity/SystemCommandInjection def config_path(service) case service @@ -99,10 +88,6 @@ module GitalySetup end end - def repos_path(storage = REPOS_STORAGE) - Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path - end - def service_binary(service) case service when :gitaly, :gitaly2 @@ -211,104 +196,4 @@ module GitalySetup raise "could not connect to #{socket}" end - - def gitaly_socket_path - Gitlab::GitalyClient.address(REPOS_STORAGE).delete_prefix('unix:') - end - - def gitaly_dir - socket_path = gitaly_socket_path - socket_path = File.expand_path(gitaly_socket_path) if expand_path_for_socket? - - File.dirname(socket_path) - end - - # Linux fails with "bind: invalid argument" if a UNIX socket path exceeds 108 characters: - # https://github.com/golang/go/issues/6895. We use absolute paths in CI to ensure - # that changes in the current working directory don't affect GRPC reconnections. - def expand_path_for_socket? - !!ENV['CI'] - end - - def setup_gitaly - unless ENV['CI'] - # In CI Gitaly is built in the setup-test-env job and saved in the - # artifacts. So when tests are started, there's no need to build Gitaly. - build_gitaly - end - - Gitlab::SetupHelper::Gitaly.create_configuration( - gitaly_dir, - { 'default' => repos_path }, - force: true, - options: { - prometheus_listen_addr: 'localhost:9236' - } - ) - Gitlab::SetupHelper::Gitaly.create_configuration( - gitaly_dir, - { 'default' => repos_path }, - force: true, - options: { - internal_socket_dir: File.join(gitaly_dir, "internal_gitaly2"), - gitaly_socket: "gitaly2.socket", - config_filename: "gitaly2.config.toml" - } - ) - Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true) - end - - def socket_path(service) - File.join(tmp_tests_gitaly_dir, "#{service}.socket") - end - - def praefect_socket_path - "unix:" + socket_path(:praefect) - end - - def wait(service) - sleep_time = 10 - sleep_interval = 0.1 - socket = socket_path(service) - - Integer(sleep_time / sleep_interval).times do - Socket.unix(socket) - return - rescue StandardError - sleep sleep_interval - end - - raise "could not connect to #{service} at #{socket.inspect} after #{sleep_time} seconds" - end - - def stop(pid) - Process.kill('KILL', pid) - rescue Errno::ESRCH - # The process can already be gone if the test run was INTerrupted. - end - - def spawn_gitaly - check_gitaly_config! - - gitaly_pid = start_gitaly - gitaly2_pid = start_gitaly2 - praefect_pid = start_praefect - - Kernel.at_exit do - # In CI this function is called by scripts/gitaly-test-spawn, triggered a - # before_script. Gitaly needs to remain running until the container is - # stopped. - next if ENV['CI'] - - pids = [gitaly_pid, gitaly2_pid, praefect_pid] - pids.each { |pid| stop(pid) } - end - - wait('gitaly') - wait('praefect') - rescue StandardError - message = 'gitaly spawn failed' - message += " (try `rm -rf #{gitaly_dir}` ?)" unless ENV['CI'] - raise message - end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index ecd2baa0ca6..294e5bdd085 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'parallel' -require_relative 'gitaly_setup' module TestEnv extend self @@ -94,6 +93,7 @@ module TestEnv }.freeze TMP_TEST_PATH = Rails.root.join('tmp', 'tests').freeze + REPOS_STORAGE = 'default' SECOND_STORAGE_PATH = Rails.root.join('tmp', 'tests', 'second_storage') SETUP_METHODS = %i[setup_gitaly setup_gitlab_shell setup_workhorse setup_factory_repo setup_forked_repo].freeze @@ -128,7 +128,7 @@ module TestEnv # Can be overriden def post_init - start_gitaly + start_gitaly(gitaly_dir) end # Clean /tmp/tests @@ -142,7 +142,7 @@ module TestEnv end FileUtils.mkdir_p( - Gitlab::GitalyClient::StorageSettings.allow_disk_access { GitalySetup.repos_path } + Gitlab::GitalyClient::StorageSettings.allow_disk_access { TestEnv.repos_path } ) FileUtils.mkdir_p(SECOND_STORAGE_PATH) FileUtils.mkdir_p(backup_path) @@ -158,28 +158,111 @@ module TestEnv def setup_gitaly component_timed_setup('Gitaly', - install_dir: GitalySetup.gitaly_dir, + install_dir: gitaly_dir, version: Gitlab::GitalyClient.expected_server_version, - task: "gitlab:gitaly:clone", - fresh_install: ENV.key?('FORCE_GITALY_INSTALL'), - task_args: [GitalySetup.gitaly_dir, GitalySetup.repos_path, gitaly_url].compact) do - GitalySetup.setup_gitaly - end + task: "gitlab:gitaly:test_install", + task_args: [gitaly_dir, repos_path, gitaly_url].compact) do + Gitlab::SetupHelper::Gitaly.create_configuration( + gitaly_dir, + { 'default' => repos_path }, + force: true, + options: { + prometheus_listen_addr: 'localhost:9236' + } + ) + Gitlab::SetupHelper::Gitaly.create_configuration( + gitaly_dir, + { 'default' => repos_path }, + force: true, + options: { + internal_socket_dir: File.join(gitaly_dir, "internal_gitaly2"), + gitaly_socket: "gitaly2.socket", + config_filename: "gitaly2.config.toml" + } + ) + Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true) + end + end + + def gitaly_socket_path + Gitlab::GitalyClient.address('default').sub(/\Aunix:/, '') + end + + def gitaly_dir + socket_path = gitaly_socket_path + socket_path = File.expand_path(gitaly_socket_path) if expand_path? + + File.dirname(socket_path) end - def start_gitaly + # Linux fails with "bind: invalid argument" if a UNIX socket path exceeds 108 characters: + # https://github.com/golang/go/issues/6895. We use absolute paths in CI to ensure + # that changes in the current working directory don't affect GRPC reconnections. + def expand_path? + !!ENV['CI'] + end + + def start_gitaly(gitaly_dir) if ci? # Gitaly has been spawned outside this process already return end - GitalySetup.spawn_gitaly + spawn_script = Rails.root.join('scripts/gitaly-test-spawn').to_s + Bundler.with_original_env do + unless system(spawn_script) + message = 'gitaly spawn failed' + message += " (try `rm -rf #{gitaly_dir}` ?)" unless ci? + raise message + end + end + + gitaly_pid = Integer(File.read(TMP_TEST_PATH.join('gitaly.pid'))) + gitaly2_pid = Integer(File.read(TMP_TEST_PATH.join('gitaly2.pid'))) + praefect_pid = Integer(File.read(TMP_TEST_PATH.join('praefect.pid'))) + + Kernel.at_exit do + pids = [gitaly_pid, gitaly2_pid, praefect_pid] + pids.each { |pid| stop(pid) } + end + + wait('gitaly') + wait('praefect') + end + + def stop(pid) + Process.kill('KILL', pid) + rescue Errno::ESRCH + # The process can already be gone if the test run was INTerrupted. end def gitaly_url ENV.fetch('GITALY_REPO_URL', nil) end + def socket_path(service) + TMP_TEST_PATH.join('gitaly', "#{service}.socket").to_s + end + + def praefect_socket_path + "unix:" + socket_path(:praefect) + end + + def wait(service) + sleep_time = 10 + sleep_interval = 0.1 + socket = socket_path(service) + + Integer(sleep_time / sleep_interval).times do + Socket.unix(socket) + return + rescue StandardError + sleep sleep_interval + end + + raise "could not connect to #{service} at #{socket.inspect} after #{sleep_time} seconds" + end + # Feature specs are run through Workhorse def setup_workhorse # Always rebuild the config file @@ -295,7 +378,8 @@ module TestEnv def rm_storage_dir(storage, dir) Gitlab::GitalyClient::StorageSettings.allow_disk_access do - target_repo_refs_path = File.join(GitalySetup.repos_path(storage), dir) + repos_path = Gitlab.config.repositories.storages[storage].legacy_disk_path + target_repo_refs_path = File.join(repos_path, dir) FileUtils.remove_dir(target_repo_refs_path) end rescue Errno::ENOENT @@ -303,7 +387,8 @@ module TestEnv def storage_dir_exists?(storage, dir) Gitlab::GitalyClient::StorageSettings.allow_disk_access do - File.exist?(File.join(GitalySetup.repos_path(storage), dir)) + repos_path = Gitlab.config.repositories.storages[storage].legacy_disk_path + File.exist?(File.join(repos_path, dir)) end end @@ -316,7 +401,7 @@ module TestEnv end def repos_path - @repos_path ||= GitalySetup.repos_path + @repos_path ||= Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path end def backup_path @@ -437,7 +522,7 @@ module TestEnv end end - def component_timed_setup(component, install_dir:, version:, task:, fresh_install: true, task_args: []) + def component_timed_setup(component, install_dir:, version:, task:, task_args: []) start = Time.now ensure_component_dir_name_is_correct!(component, install_dir) @@ -447,7 +532,7 @@ module TestEnv if component_needs_update?(install_dir, version) # Cleanup the component entirely to ensure we start fresh - FileUtils.rm_rf(install_dir) if fresh_install + FileUtils.rm_rf(install_dir) if ENV['SKIP_RAILS_ENV_IN_RAKE'] # When we run `scripts/setup-test-env`, we take care of loading the necessary dependencies diff --git a/spec/support/praefect.rb b/spec/support/praefect.rb index 451b47cc83c..3218275c2aa 100644 --- a/spec/support/praefect.rb +++ b/spec/support/praefect.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -require_relative 'helpers/gitaly_setup' +require_relative 'helpers/test_env' RSpec.configure do |config| config.before(:each, :praefect) do allow(Gitlab.config.repositories.storages['default']).to receive(:[]).and_call_original allow(Gitlab.config.repositories.storages['default']).to receive(:[]).with('gitaly_address') - .and_return(GitalySetup.praefect_socket_path) + .and_return(TestEnv.praefect_socket_path) end end diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index c6833531f62..27e9da3c6df 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -432,7 +432,7 @@ RSpec.describe 'gitlab:app namespace rake task', :delete do .with(max_concurrency: 5, max_storage_concurrency: 2) .and_call_original end - expect(::Backup::GitalyBackup).to receive(:new).with(anything, parallel: 5, parallel_storage: 2).and_call_original + expect(::Backup::GitalyBackup).to receive(:new).with(anything, max_parallelism: 5, storage_parallelism: 2).and_call_original expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout_from_any_process end diff --git a/spec/workers/web_hook_worker_spec.rb b/spec/workers/web_hook_worker_spec.rb index 0f40177eb7d..bbb8844a447 100644 --- a/spec/workers/web_hook_worker_spec.rb +++ b/spec/workers/web_hook_worker_spec.rb @@ -19,6 +19,15 @@ RSpec.describe WebHookWorker do expect { subject.perform(non_existing_record_id, data, hook_name) }.not_to raise_error end + it 'retrieves recursion detection data, reinstates it, and cleans it from payload', :request_store, :aggregate_failures do + uuid = SecureRandom.uuid + full_data = data.merge({ _gitlab_recursion_detection_request_uuid: uuid }) + + expect_next(WebHookService, project_hook, data.with_indifferent_access, hook_name, anything).to receive(:execute) + expect { subject.perform(project_hook.id, full_data, hook_name) } + .to change { Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid }.to(uuid) + end + it_behaves_like 'worker with data consistency', described_class, data_consistency: :delayed |