diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-04 15:10:25 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-04 15:10:25 +0000 |
| commit | d4194db620cc5b736bb5737ed5e4eab979ccd7ab (patch) | |
| tree | 2aab61db9bde950c45f93f43f4033231fb956528 | |
| parent | c80a1141e306596202f694b101bfb1aab1864de9 (diff) | |
| download | gitlab-ce-d4194db620cc5b736bb5737ed5e4eab979ccd7ab.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
66 files changed, 1324 insertions, 758 deletions
diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index ce0b2795686..dc18eab7e57 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -13.18.0 +13.19.0 @@ -301,9 +301,7 @@ gem 'gitlab-license', '~> 1.5' gem 'rack-attack', '~> 6.3.0' # Sentry integration -gem 'sentry-ruby', '~> 4.4.0' -gem 'sentry-sidekiq', '~> 4.4.0' -gem 'sentry-rails', '~> 4.4.0' +gem 'sentry-raven', '~> 3.1' # PostgreSQL query parsing # diff --git a/Gemfile.lock b/Gemfile.lock index 837edfffdf0..59cbd84a3e6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1172,18 +1172,8 @@ GEM selenium-webdriver (3.142.7) childprocess (>= 0.5, < 4.0) rubyzip (>= 1.2.2) - sentry-rails (4.4.0) - railties (>= 5.0) - sentry-ruby-core (~> 4.4.0.pre.beta) - sentry-ruby (4.4.2) - concurrent-ruby (~> 1.0, >= 1.0.2) + sentry-raven (3.1.2) faraday (>= 1.0) - sentry-ruby-core (= 4.4.2) - sentry-ruby-core (4.4.2) - concurrent-ruby - faraday - sentry-sidekiq (4.4.0) - sentry-ruby-core (~> 4.4.0.pre.beta) settingslogic (2.0.9) sexp_processor (4.15.1) shellany (0.0.1) @@ -1629,9 +1619,7 @@ DEPENDENCIES sassc-rails (~> 2.1.0) seed-fu (~> 2.3.7) selenium-webdriver (~> 3.142) - sentry-rails (~> 4.4.0) - sentry-ruby (~> 4.4.0) - sentry-sidekiq (~> 4.4.0) + sentry-raven (~> 3.1) settingslogic (~> 2.0.9) shoulda-matchers (~> 4.0.1) sidekiq (~> 5.2.7) diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index f5b2d266c18..f9b474f9ec9 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -46,7 +46,7 @@ export function initMermaid(mermaid) { theme, flowchart: { useMaxWidth: true, - htmlLabels: false, + htmlLabels: true, }, securityLevel: 'strict', }); diff --git a/app/assets/javascripts/pages/projects/index.js b/app/assets/javascripts/pages/projects/index.js index 45e9643b3f3..1eab3becbc3 100644 --- a/app/assets/javascripts/pages/projects/index.js +++ b/app/assets/javascripts/pages/projects/index.js @@ -1,5 +1,7 @@ import ShortcutsNavigation from '../../behaviors/shortcuts/shortcuts_navigation'; +import { initSidebarTracking } from '../shared/nav/sidebar_tracking'; import Project from './project'; new Project(); // eslint-disable-line no-new new ShortcutsNavigation(); // eslint-disable-line no-new +initSidebarTracking(); diff --git a/app/assets/javascripts/pages/shared/nav/sidebar_tracking.js b/app/assets/javascripts/pages/shared/nav/sidebar_tracking.js new file mode 100644 index 00000000000..79ce1a37d21 --- /dev/null +++ b/app/assets/javascripts/pages/shared/nav/sidebar_tracking.js @@ -0,0 +1,44 @@ +function onSidebarLinkClick() { + const setDataTrackAction = (element, action) => { + element.setAttribute('data-track-action', action); + }; + + const setDataTrackExtra = (element, value) => { + const SIDEBAR_COLLAPSED = 'Collapsed'; + const SIDEBAR_EXPANDED = 'Expanded'; + const sidebarCollapsed = document + .querySelector('.nav-sidebar') + .classList.contains('js-sidebar-collapsed') + ? SIDEBAR_COLLAPSED + : SIDEBAR_EXPANDED; + + element.setAttribute( + 'data-track-extra', + JSON.stringify({ sidebar_display: sidebarCollapsed, menu_display: value }), + ); + }; + + const EXPANDED = 'Expanded'; + const FLY_OUT = 'Fly out'; + const CLICK_MENU_ACTION = 'click_menu'; + const CLICK_MENU_ITEM_ACTION = 'click_menu_item'; + const parentElement = this.parentNode; + const subMenuList = parentElement.closest('.sidebar-sub-level-items'); + + if (subMenuList) { + const isFlyOut = subMenuList.classList.contains('fly-out-list') ? FLY_OUT : EXPANDED; + + setDataTrackExtra(parentElement, isFlyOut); + setDataTrackAction(parentElement, CLICK_MENU_ITEM_ACTION); + } else { + const isFlyOut = parentElement.classList.contains('is-showing-fly-out') ? FLY_OUT : EXPANDED; + + setDataTrackExtra(parentElement, isFlyOut); + setDataTrackAction(parentElement, CLICK_MENU_ACTION); + } +} +export const initSidebarTracking = () => { + document.querySelectorAll('.nav-sidebar li[data-track-label] > a').forEach((link) => { + link.addEventListener('click', onSidebarLinkClick); + }); +}; diff --git a/app/assets/javascripts/pipelines/components/graph_shared/drawing_utils.js b/app/assets/javascripts/pipelines/components/graph_shared/drawing_utils.js index 7c306683305..7c62acbe8de 100644 --- a/app/assets/javascripts/pipelines/components/graph_shared/drawing_utils.js +++ b/app/assets/javascripts/pipelines/components/graph_shared/drawing_utils.js @@ -60,8 +60,16 @@ export const generateLinksData = ({ links }, containerID, modifier = '') => { paddingTop + sourceNodeCoordinates.height / 2; - // Start point - path.moveTo(sourceNodeX, sourceNodeY); + const sourceNodeLeftX = sourceNodeCoordinates.left - containerCoordinates.x - paddingLeft; + + // If the source and target X values are the same, + // it means the nodes are in the same column so we + // want to start the line on the left of the pill + // instead of the right to have a nice curve. + const firstPointCoordinateX = sourceNodeLeftX === targetNodeX ? sourceNodeLeftX : sourceNodeX; + + // First point + path.moveTo(firstPointCoordinateX, sourceNodeY); // Make cross-stages lines a straight line all the way // until we can safely draw the bezier to look nice. diff --git a/app/assets/javascripts/pipelines/components/notification/pipeline_notification.vue b/app/assets/javascripts/pipelines/components/notification/pipeline_notification.vue deleted file mode 100644 index 6dff3828a34..00000000000 --- a/app/assets/javascripts/pipelines/components/notification/pipeline_notification.vue +++ /dev/null @@ -1,90 +0,0 @@ -<script> -import { GlBanner, GlLink, GlSprintf } from '@gitlab/ui'; -import createFlash from '~/flash'; -import { __ } from '~/locale'; -import DismissPipelineGraphCallout from '../../graphql/mutations/dismiss_pipeline_notification.graphql'; -import getUserCallouts from '../../graphql/queries/get_user_callouts.query.graphql'; - -const featureName = 'pipeline_needs_banner'; -const enumFeatureName = featureName.toUpperCase(); - -export default { - i18n: { - title: __('View job dependencies in the pipeline graph!'), - description: __( - 'You can now group jobs in the pipeline graph based on which jobs are configured to run first, if you use the %{codeStart}needs:%{codeEnd} keyword to establish job dependencies in your CI/CD pipelines. %{linkStart}Learn how to speed up your pipeline with needs.%{linkEnd}', - ), - buttonText: __('Provide feedback'), - }, - components: { - GlBanner, - GlLink, - GlSprintf, - }, - apollo: { - callouts: { - query: getUserCallouts, - update(data) { - return data?.currentUser?.callouts?.nodes.map((c) => c.featureName); - }, - error() { - this.hasError = true; - }, - }, - }, - inject: ['dagDocPath'], - data() { - return { - callouts: [], - dismissedAlert: false, - hasError: false, - }; - }, - computed: { - showBanner() { - return ( - !this.$apollo.queries.callouts?.loading && - !this.hasError && - !this.dismissedAlert && - !this.callouts.includes(enumFeatureName) - ); - }, - }, - methods: { - handleClose() { - this.dismissedAlert = true; - try { - this.$apollo.mutate({ - mutation: DismissPipelineGraphCallout, - variables: { - featureName, - }, - }); - } catch { - createFlash(__('There was a problem dismissing this notification.')); - } - }, - }, -}; -</script> -<template> - <gl-banner - v-if="showBanner" - :title="$options.i18n.title" - :button-text="$options.i18n.buttonText" - button-link="https://gitlab.com/gitlab-org/gitlab/-/issues/327688" - variant="introduction" - @close="handleClose" - > - <p> - <gl-sprintf :message="$options.i18n.description"> - <template #link="{ content }"> - <gl-link :href="dagDocPath" target="_blank"> {{ content }}</gl-link> - </template> - <template #code="{ content }"> - <code>{{ content }}</code> - </template> - </gl-sprintf> - </p> - </gl-banner> -</template> diff --git a/app/assets/javascripts/pipelines/pipeline_details_bundle.js b/app/assets/javascripts/pipelines/pipeline_details_bundle.js index 911f40f4db3..9ab4753fec8 100644 --- a/app/assets/javascripts/pipelines/pipeline_details_bundle.js +++ b/app/assets/javascripts/pipelines/pipeline_details_bundle.js @@ -9,7 +9,6 @@ import GraphBundleMixin from './mixins/graph_pipeline_bundle_mixin'; import createDagApp from './pipeline_details_dag'; import { createPipelinesDetailApp } from './pipeline_details_graph'; import { createPipelineHeaderApp } from './pipeline_details_header'; -import { createPipelineNotificationApp } from './pipeline_details_notification'; import { apolloProvider } from './pipeline_shared_client'; import createTestReportsStore from './stores/test_reports'; import { reportToSentry } from './utils'; @@ -20,7 +19,6 @@ const SELECTORS = { PIPELINE_DETAILS: '.js-pipeline-details-vue', PIPELINE_GRAPH: '#js-pipeline-graph-vue', PIPELINE_HEADER: '#js-pipeline-header-vue', - PIPELINE_NOTIFICATION: '#js-pipeline-notification', PIPELINE_TESTS: '#js-pipeline-tests-detail', }; @@ -101,14 +99,6 @@ export default async function initPipelineDetailsBundle() { Flash(__('An error occurred while loading a section of this page.')); } - if (gon.features.pipelineGraphLayersView) { - try { - createPipelineNotificationApp(SELECTORS.PIPELINE_NOTIFICATION, apolloProvider); - } catch { - Flash(__('An error occurred while loading a section of this page.')); - } - } - if (canShowNewPipelineDetails) { try { createPipelinesDetailApp(SELECTORS.PIPELINE_GRAPH, apolloProvider, dataset); diff --git a/app/assets/javascripts/pipelines/pipeline_details_notification.js b/app/assets/javascripts/pipelines/pipeline_details_notification.js deleted file mode 100644 index be234e8972d..00000000000 --- a/app/assets/javascripts/pipelines/pipeline_details_notification.js +++ /dev/null @@ -1,29 +0,0 @@ -import Vue from 'vue'; -import VueApollo from 'vue-apollo'; -import PipelineNotification from './components/notification/pipeline_notification.vue'; - -Vue.use(VueApollo); - -export const createPipelineNotificationApp = (elSelector, apolloProvider) => { - const el = document.querySelector(elSelector); - - if (!el) { - return; - } - - const { dagDocPath } = el?.dataset; - // eslint-disable-next-line no-new - new Vue({ - el, - components: { - PipelineNotification, - }, - provide: { - dagDocPath, - }, - apolloProvider, - render(createElement) { - return createElement('pipeline-notification'); - }, - }); -}; diff --git a/app/assets/stylesheets/startup/startup-dark.scss b/app/assets/stylesheets/startup/startup-dark.scss index 92927fecd09..dafa89465d1 100644 --- a/app/assets/stylesheets/startup/startup-dark.scss +++ b/app/assets/stylesheets/startup/startup-dark.scss @@ -930,7 +930,7 @@ input { } @media (min-width: 768px) { .page-with-contextual-sidebar { - padding-left: 50px; + padding-left: 48px; } } @media (min-width: 1200px) { @@ -940,7 +940,7 @@ input { } @media (min-width: 768px) { .page-with-icon-sidebar { - padding-left: 50px; + padding-left: 48px; } } .nav-sidebar { @@ -960,7 +960,7 @@ input { } } .nav-sidebar.sidebar-collapsed-desktop { - width: 50px; + width: 48px; } .nav-sidebar.sidebar-collapsed-desktop .nav-sidebar-inner-scroll { overflow-x: hidden; @@ -1026,7 +1026,7 @@ input { } @media (min-width: 768px) and (max-width: 1199px) { .nav-sidebar:not(.sidebar-expanded-mobile) { - width: 50px; + width: 48px; } .nav-sidebar:not(.sidebar-expanded-mobile) .nav-sidebar-inner-scroll { overflow-x: hidden; @@ -1055,7 +1055,7 @@ input { } .nav-sidebar:not(.sidebar-expanded-mobile) .context-header { height: 60px; - width: 50px; + width: 48px; } .nav-sidebar:not(.sidebar-expanded-mobile) .context-header a { padding: 10px 4px; @@ -1082,7 +1082,7 @@ input { } .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button { padding: 16px; - width: 49px; + width: 47px; } .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button @@ -1172,7 +1172,7 @@ input { } .sidebar-collapsed-desktop .context-header { height: 60px; - width: 50px; + width: 48px; } .sidebar-collapsed-desktop .context-header a { padding: 10px 4px; @@ -1199,7 +1199,7 @@ input { } .sidebar-collapsed-desktop .toggle-sidebar-button { padding: 16px; - width: 49px; + width: 47px; } .sidebar-collapsed-desktop .toggle-sidebar-button .collapse-text, .sidebar-collapsed-desktop .toggle-sidebar-button .icon-chevron-double-lg-left { @@ -1273,7 +1273,7 @@ body.sidebar-refactoring .nav-sidebar li.active { } @media (min-width: 768px) { body.sidebar-refactoring .page-with-contextual-sidebar { - padding-left: 50px; + padding-left: 48px; } } @media (min-width: 1200px) { @@ -1283,26 +1283,21 @@ body.sidebar-refactoring .nav-sidebar li.active { } @media (min-width: 768px) { body.sidebar-refactoring .page-with-icon-sidebar { - padding-left: 50px; + padding-left: 48px; } } body.sidebar-refactoring .nav-sidebar { position: fixed; + bottom: 0; + left: 0; z-index: 600; width: 220px; top: 40px; - bottom: 0; - left: 0; background-color: #303030; transform: translate3d(0, 0, 0); } -@media (min-width: 576px) and (max-width: 576px) { - body.sidebar-refactoring .nav-sidebar:not(.sidebar-collapsed-desktop) { - box-shadow: inset -1px 0 0 #404040, 2px 1px 3px rgba(0, 0, 0, 0.1); - } -} body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop { - width: 50px; + width: 48px; } body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop @@ -1312,7 +1307,8 @@ body.sidebar-refactoring body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop .badge.badge-pill:not(.fly-out-badge), -body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop .nav-item-name { +body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop .nav-item-name, +body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop .collapse-text { border: 0; clip: rect(0, 0, 0, 0); height: 1px; @@ -1328,7 +1324,7 @@ body.sidebar-refactoring .sidebar-top-level-items > li > a { - min-height: 45px; + min-height: unset; } body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop @@ -1340,6 +1336,9 @@ body.sidebar-refactoring .avatar-container { margin: 0 auto; } +body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop li.active > a { + background-color: rgba(41, 41, 97, 0.08); +} body.sidebar-refactoring .nav-sidebar a { text-decoration: none; line-height: 1rem; @@ -1386,7 +1385,7 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { } @media (min-width: 768px) and (max-width: 1199px) { body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) { - width: 50px; + width: 48px; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) @@ -1398,7 +1397,10 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { .badge.badge-pill:not(.fly-out-badge), body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) - .nav-item-name { + .nav-item-name, + body.sidebar-refactoring + .nav-sidebar:not(.sidebar-expanded-mobile) + .collapse-text { border: 0; clip: rect(0, 0, 0, 0); height: 1px; @@ -1414,7 +1416,7 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { .sidebar-top-level-items > li > a { - min-height: 45px; + min-height: unset; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) @@ -1428,9 +1430,15 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) + li.active + > a { + background-color: rgba(41, 41, 97, 0.08); + } + body.sidebar-refactoring + .nav-sidebar:not(.sidebar-expanded-mobile) .context-header { height: 60px; - width: 50px; + width: 48px; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) @@ -1453,6 +1461,17 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) + .context-header { + height: auto; + } + body.sidebar-refactoring + .nav-sidebar:not(.sidebar-expanded-mobile) + .context-header + a { + padding: 0.25rem; + } + body.sidebar-refactoring + .nav-sidebar:not(.sidebar-expanded-mobile) .sidebar-top-level-items > li .sidebar-sub-level-items:not(.flyout-list) { @@ -1466,23 +1485,19 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button { - padding: 16px; - width: 49px; + width: 48px; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button - .collapse-text, - body.sidebar-refactoring - .nav-sidebar:not(.sidebar-expanded-mobile) - .toggle-sidebar-button - .icon-chevron-double-lg-left { + .collapse-text { display: none; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button - .icon-chevron-double-lg-right { + .icon-chevron-double-lg-left { + transform: rotate(180deg); display: block; margin: 0; } @@ -1507,6 +1522,7 @@ body.sidebar-refactoring .nav-sidebar-inner-scroll > div.context-header a { margin: 1px 4px; padding: 0.25rem; margin-bottom: 0.25rem; + margin-top: 0; } body.sidebar-refactoring .nav-sidebar-inner-scroll @@ -1547,6 +1563,7 @@ body.sidebar-refactoring .sidebar-top-level-items { body.sidebar-refactoring .sidebar-top-level-items .context-header a { padding: 0.25rem; margin-bottom: 0.25rem; + margin-top: 0; } body.sidebar-refactoring .sidebar-top-level-items @@ -1619,7 +1636,11 @@ body.sidebar-refactoring .close-nav-button { display: flex; align-items: center; background-color: #303030; + border-top: 1px solid #404040; color: #2f2a6b; + position: fixed; + bottom: 0; + width: 220px; } body.sidebar-refactoring .toggle-sidebar-button .collapse-text, body.sidebar-refactoring .toggle-sidebar-button .icon-chevron-double-lg-left, @@ -1629,28 +1650,13 @@ body.sidebar-refactoring .close-nav-button .icon-chevron-double-lg-left, body.sidebar-refactoring .close-nav-button .icon-chevron-double-lg-right { color: inherit; } -body.sidebar-refactoring .toggle-sidebar-button, -body.sidebar-refactoring .close-nav-button { - position: fixed; - bottom: 0; - width: 219px; - border-top: 1px solid #404040; -} -body.sidebar-refactoring .toggle-sidebar-button svg, -body.sidebar-refactoring .close-nav-button svg { - margin-right: 8px; -} -body.sidebar-refactoring .toggle-sidebar-button .icon-chevron-double-lg-right, -body.sidebar-refactoring .close-nav-button .icon-chevron-double-lg-right { - display: none; -} body.sidebar-refactoring .collapse-text { white-space: nowrap; overflow: hidden; } body.sidebar-refactoring .sidebar-collapsed-desktop .context-header { height: 60px; - width: 50px; + width: 48px; } body.sidebar-refactoring .sidebar-collapsed-desktop .context-header a { padding: 10px 4px; @@ -1666,6 +1672,12 @@ body.sidebar-refactoring .sidebar-collapsed-desktop .sidebar-context-title { white-space: nowrap; width: 1px; } +body.sidebar-refactoring .sidebar-collapsed-desktop .context-header { + height: auto; +} +body.sidebar-refactoring .sidebar-collapsed-desktop .context-header a { + padding: 0.25rem; +} body.sidebar-refactoring .sidebar-collapsed-desktop .sidebar-top-level-items @@ -1677,23 +1689,19 @@ body.sidebar-refactoring .sidebar-collapsed-desktop .nav-icon-container { margin-right: 0; } body.sidebar-refactoring .sidebar-collapsed-desktop .toggle-sidebar-button { - padding: 16px; - width: 49px; + width: 48px; } body.sidebar-refactoring .sidebar-collapsed-desktop .toggle-sidebar-button - .collapse-text, -body.sidebar-refactoring - .sidebar-collapsed-desktop - .toggle-sidebar-button - .icon-chevron-double-lg-left { + .collapse-text { display: none; } body.sidebar-refactoring .sidebar-collapsed-desktop .toggle-sidebar-button - .icon-chevron-double-lg-right { + .icon-chevron-double-lg-left { + transform: rotate(180deg); display: block; margin: 0; } @@ -1834,6 +1842,12 @@ svg.s16 { height: 32px; margin-right: 8px; } +.avatar.s40, +.avatar-container.s40 { + width: 40px; + height: 40px; + margin-right: 8px; +} .avatar { transition-property: none; width: 40px; @@ -1861,6 +1875,10 @@ svg.s16 { font-size: 14px; line-height: 32px; } +.identicon.s40 { + font-size: 16px; + line-height: 38px; +} .identicon.bg1 { background-color: #ffebee; } @@ -1900,6 +1918,10 @@ svg.s16 { margin: 0; align-self: center; } +.avatar-container.s40 { + min-width: 40px; + min-height: 40px; +} .rect-avatar { border-radius: 2px; } @@ -1924,6 +1946,9 @@ body.sidebar-refactoring .avatar.s32 { border-radius: 4px; } +.rect-avatar.s40 { + border-radius: 4px; +} body.gl-dark .navbar-gitlab { background-color: #fafafa; } diff --git a/app/assets/stylesheets/startup/startup-general.scss b/app/assets/stylesheets/startup/startup-general.scss index abc6a6aec1c..c7020e8af55 100644 --- a/app/assets/stylesheets/startup/startup-general.scss +++ b/app/assets/stylesheets/startup/startup-general.scss @@ -915,7 +915,7 @@ input { } @media (min-width: 768px) { .page-with-contextual-sidebar { - padding-left: 50px; + padding-left: 48px; } } @media (min-width: 1200px) { @@ -925,7 +925,7 @@ input { } @media (min-width: 768px) { .page-with-icon-sidebar { - padding-left: 50px; + padding-left: 48px; } } .nav-sidebar { @@ -945,7 +945,7 @@ input { } } .nav-sidebar.sidebar-collapsed-desktop { - width: 50px; + width: 48px; } .nav-sidebar.sidebar-collapsed-desktop .nav-sidebar-inner-scroll { overflow-x: hidden; @@ -1011,7 +1011,7 @@ input { } @media (min-width: 768px) and (max-width: 1199px) { .nav-sidebar:not(.sidebar-expanded-mobile) { - width: 50px; + width: 48px; } .nav-sidebar:not(.sidebar-expanded-mobile) .nav-sidebar-inner-scroll { overflow-x: hidden; @@ -1040,7 +1040,7 @@ input { } .nav-sidebar:not(.sidebar-expanded-mobile) .context-header { height: 60px; - width: 50px; + width: 48px; } .nav-sidebar:not(.sidebar-expanded-mobile) .context-header a { padding: 10px 4px; @@ -1067,7 +1067,7 @@ input { } .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button { padding: 16px; - width: 49px; + width: 47px; } .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button @@ -1157,7 +1157,7 @@ input { } .sidebar-collapsed-desktop .context-header { height: 60px; - width: 50px; + width: 48px; } .sidebar-collapsed-desktop .context-header a { padding: 10px 4px; @@ -1184,7 +1184,7 @@ input { } .sidebar-collapsed-desktop .toggle-sidebar-button { padding: 16px; - width: 49px; + width: 47px; } .sidebar-collapsed-desktop .toggle-sidebar-button .collapse-text, .sidebar-collapsed-desktop .toggle-sidebar-button .icon-chevron-double-lg-left { @@ -1235,7 +1235,7 @@ body.sidebar-refactoring .nav-sidebar li.active { } @media (min-width: 768px) { body.sidebar-refactoring .page-with-contextual-sidebar { - padding-left: 50px; + padding-left: 48px; } } @media (min-width: 1200px) { @@ -1245,26 +1245,21 @@ body.sidebar-refactoring .nav-sidebar li.active { } @media (min-width: 768px) { body.sidebar-refactoring .page-with-icon-sidebar { - padding-left: 50px; + padding-left: 48px; } } body.sidebar-refactoring .nav-sidebar { position: fixed; + bottom: 0; + left: 0; z-index: 600; width: 220px; top: 40px; - bottom: 0; - left: 0; background-color: #f0f0f0; transform: translate3d(0, 0, 0); } -@media (min-width: 576px) and (max-width: 576px) { - body.sidebar-refactoring .nav-sidebar:not(.sidebar-collapsed-desktop) { - box-shadow: inset -1px 0 0 #dbdbdb, 2px 1px 3px rgba(0, 0, 0, 0.1); - } -} body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop { - width: 50px; + width: 48px; } body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop @@ -1274,7 +1269,8 @@ body.sidebar-refactoring body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop .badge.badge-pill:not(.fly-out-badge), -body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop .nav-item-name { +body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop .nav-item-name, +body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop .collapse-text { border: 0; clip: rect(0, 0, 0, 0); height: 1px; @@ -1290,7 +1286,7 @@ body.sidebar-refactoring .sidebar-top-level-items > li > a { - min-height: 45px; + min-height: unset; } body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop @@ -1302,6 +1298,9 @@ body.sidebar-refactoring .avatar-container { margin: 0 auto; } +body.sidebar-refactoring .nav-sidebar.sidebar-collapsed-desktop li.active > a { + background-color: rgba(41, 41, 97, 0.08); +} body.sidebar-refactoring .nav-sidebar a { text-decoration: none; line-height: 1rem; @@ -1348,7 +1347,7 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { } @media (min-width: 768px) and (max-width: 1199px) { body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) { - width: 50px; + width: 48px; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) @@ -1360,7 +1359,10 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { .badge.badge-pill:not(.fly-out-badge), body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) - .nav-item-name { + .nav-item-name, + body.sidebar-refactoring + .nav-sidebar:not(.sidebar-expanded-mobile) + .collapse-text { border: 0; clip: rect(0, 0, 0, 0); height: 1px; @@ -1376,7 +1378,7 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { .sidebar-top-level-items > li > a { - min-height: 45px; + min-height: unset; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) @@ -1390,9 +1392,15 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) + li.active + > a { + background-color: rgba(41, 41, 97, 0.08); + } + body.sidebar-refactoring + .nav-sidebar:not(.sidebar-expanded-mobile) .context-header { height: 60px; - width: 50px; + width: 48px; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) @@ -1415,6 +1423,17 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) + .context-header { + height: auto; + } + body.sidebar-refactoring + .nav-sidebar:not(.sidebar-expanded-mobile) + .context-header + a { + padding: 0.25rem; + } + body.sidebar-refactoring + .nav-sidebar:not(.sidebar-expanded-mobile) .sidebar-top-level-items > li .sidebar-sub-level-items:not(.flyout-list) { @@ -1428,23 +1447,19 @@ body.sidebar-refactoring .nav-sidebar .fly-out-top-item { body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button { - padding: 16px; - width: 49px; + width: 48px; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button - .collapse-text, - body.sidebar-refactoring - .nav-sidebar:not(.sidebar-expanded-mobile) - .toggle-sidebar-button - .icon-chevron-double-lg-left { + .collapse-text { display: none; } body.sidebar-refactoring .nav-sidebar:not(.sidebar-expanded-mobile) .toggle-sidebar-button - .icon-chevron-double-lg-right { + .icon-chevron-double-lg-left { + transform: rotate(180deg); display: block; margin: 0; } @@ -1469,6 +1484,7 @@ body.sidebar-refactoring .nav-sidebar-inner-scroll > div.context-header a { margin: 1px 4px; padding: 0.25rem; margin-bottom: 0.25rem; + margin-top: 0; } body.sidebar-refactoring .nav-sidebar-inner-scroll @@ -1509,6 +1525,7 @@ body.sidebar-refactoring .sidebar-top-level-items { body.sidebar-refactoring .sidebar-top-level-items .context-header a { padding: 0.25rem; margin-bottom: 0.25rem; + margin-top: 0; } body.sidebar-refactoring .sidebar-top-level-items @@ -1581,7 +1598,11 @@ body.sidebar-refactoring .close-nav-button { display: flex; align-items: center; background-color: #f0f0f0; + border-top: 1px solid #dbdbdb; color: #2f2a6b; + position: fixed; + bottom: 0; + width: 220px; } body.sidebar-refactoring .toggle-sidebar-button .collapse-text, body.sidebar-refactoring .toggle-sidebar-button .icon-chevron-double-lg-left, @@ -1591,28 +1612,13 @@ body.sidebar-refactoring .close-nav-button .icon-chevron-double-lg-left, body.sidebar-refactoring .close-nav-button .icon-chevron-double-lg-right { color: inherit; } -body.sidebar-refactoring .toggle-sidebar-button, -body.sidebar-refactoring .close-nav-button { - position: fixed; - bottom: 0; - width: 219px; - border-top: 1px solid #dbdbdb; -} -body.sidebar-refactoring .toggle-sidebar-button svg, -body.sidebar-refactoring .close-nav-button svg { - margin-right: 8px; -} -body.sidebar-refactoring .toggle-sidebar-button .icon-chevron-double-lg-right, -body.sidebar-refactoring .close-nav-button .icon-chevron-double-lg-right { - display: none; -} body.sidebar-refactoring .collapse-text { white-space: nowrap; overflow: hidden; } body.sidebar-refactoring .sidebar-collapsed-desktop .context-header { height: 60px; - width: 50px; + width: 48px; } body.sidebar-refactoring .sidebar-collapsed-desktop .context-header a { padding: 10px 4px; @@ -1628,6 +1634,12 @@ body.sidebar-refactoring .sidebar-collapsed-desktop .sidebar-context-title { white-space: nowrap; width: 1px; } +body.sidebar-refactoring .sidebar-collapsed-desktop .context-header { + height: auto; +} +body.sidebar-refactoring .sidebar-collapsed-desktop .context-header a { + padding: 0.25rem; +} body.sidebar-refactoring .sidebar-collapsed-desktop .sidebar-top-level-items @@ -1639,23 +1651,19 @@ body.sidebar-refactoring .sidebar-collapsed-desktop .nav-icon-container { margin-right: 0; } body.sidebar-refactoring .sidebar-collapsed-desktop .toggle-sidebar-button { - padding: 16px; - width: 49px; + width: 48px; } body.sidebar-refactoring .sidebar-collapsed-desktop .toggle-sidebar-button - .collapse-text, -body.sidebar-refactoring - .sidebar-collapsed-desktop - .toggle-sidebar-button - .icon-chevron-double-lg-left { + .collapse-text { display: none; } body.sidebar-refactoring .sidebar-collapsed-desktop .toggle-sidebar-button - .icon-chevron-double-lg-right { + .icon-chevron-double-lg-left { + transform: rotate(180deg); display: block; margin: 0; } @@ -1796,6 +1804,12 @@ svg.s16 { height: 32px; margin-right: 8px; } +.avatar.s40, +.avatar-container.s40 { + width: 40px; + height: 40px; + margin-right: 8px; +} .avatar { transition-property: none; width: 40px; @@ -1823,6 +1837,10 @@ svg.s16 { font-size: 14px; line-height: 32px; } +.identicon.s40 { + font-size: 16px; + line-height: 38px; +} .identicon.bg1 { background-color: #ffebee; } @@ -1862,6 +1880,10 @@ svg.s16 { margin: 0; align-self: center; } +.avatar-container.s40 { + min-width: 40px; + min-height: 40px; +} .rect-avatar { border-radius: 2px; } @@ -1886,6 +1908,9 @@ body.sidebar-refactoring .avatar.s32 { border-radius: 4px; } +.rect-avatar.s40 { + border-radius: 4px; +} .tab-width-8 { -moz-tab-size: 8; diff --git a/app/controllers/import/bulk_imports_controller.rb b/app/controllers/import/bulk_imports_controller.rb index 48635c933af..8d5c07f52c7 100644 --- a/app/controllers/import/bulk_imports_controller.rb +++ b/app/controllers/import/bulk_imports_controller.rb @@ -78,7 +78,7 @@ class Import::BulkImportsController < ApplicationController def query_params query_params = { top_level_only: true, - min_access_level: Gitlab::Access::MAINTAINER + min_access_level: Gitlab::Access::OWNER } query_params[:search] = sanitized_filter_param if sanitized_filter_param diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 3eaabfbf33e..f125952cb97 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -163,7 +163,10 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic end def render_merge_ref_head_diff? - Gitlab::Utils.to_boolean(params[:diff_head]) && @merge_request.diffable_merge_ref? && @start_sha.nil? + params[:diff_id].blank? && + Gitlab::Utils.to_boolean(params[:diff_head]) && + @merge_request.diffable_merge_ref? && + @start_sha.nil? end def note_positions diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 1b05786cb36..2db606898b9 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -57,6 +57,9 @@ class CommitStatus < ApplicationRecord scope :in_pipelines, ->(pipelines) { where(pipeline: pipelines) } scope :eager_load_pipeline, -> { eager_load(:pipeline, project: { namespace: :route }) } scope :with_pipeline, -> { joins(:pipeline) } + scope :updated_before, ->(lookback:, timeout:) { + where('(ci_builds.created_at BETWEEN ? AND ?) AND (ci_builds.updated_at BETWEEN ? AND ?)', lookback, timeout, lookback, timeout) + } scope :for_project_paths, -> (paths) do where(project: Project.where_full_path_in(Array(paths))) diff --git a/app/models/postgresql/replication_slot.rb b/app/models/postgresql/replication_slot.rb index e23a9546cae..77b42c34ad9 100644 --- a/app/models/postgresql/replication_slot.rb +++ b/app/models/postgresql/replication_slot.rb @@ -26,7 +26,7 @@ module Postgresql "(pg_current_wal_insert_lsn(), restart_lsn)::bigint" # We force the use of a transaction here so the query always goes to the - # primary, even when using the EE DB load balancer. + # primary, even when using the DB load balancer. sizes = transaction { pluck(Arel.sql(lag_function)) } too_great = sizes.compact.count { |size| size >= max } diff --git a/app/views/projects/pipelines/show.html.haml b/app/views/projects/pipelines/show.html.haml index 75174d37d7e..1c134d914e9 100644 --- a/app/views/projects/pipelines/show.html.haml +++ b/app/views/projects/pipelines/show.html.haml @@ -28,7 +28,6 @@ - lint_link_start = '<a href="%{url}">'.html_safe % { url: lint_link_url } = s_('You can also test your %{gitlab_ci_yml} in %{lint_link_start}CI Lint%{lint_link_end}').html_safe % { gitlab_ci_yml: '.gitlab-ci.yml', lint_link_start: lint_link_start, lint_link_end: '</a>'.html_safe } - #js-pipeline-notification{ data: { dag_doc_path: help_page_path('ci/yaml/README.md', anchor: 'needs') } } = render "projects/pipelines/with_tabs", pipeline: @pipeline, stages: @stages, pipeline_has_errors: pipeline_has_errors .js-pipeline-details-vue{ data: { endpoint: project_pipeline_path(@project, @pipeline, format: :json), metrics_path: namespace_project_ci_prometheus_metrics_histograms_path(namespace_id: @project.namespace, project_id: @project, format: :json), pipeline_project_path: @project.full_path, pipeline_iid: @pipeline.iid, graphql_resource_etag: graphql_etag_pipeline_path(@pipeline) } } diff --git a/app/views/shared/nav/_sidebar_menu_item.html.haml b/app/views/shared/nav/_sidebar_menu_item.html.haml index 0b0e4c7aec9..674ce593ee2 100644 --- a/app/views/shared/nav/_sidebar_menu_item.html.haml +++ b/app/views/shared/nav/_sidebar_menu_item.html.haml @@ -1,4 +1,4 @@ -= nav_link(**sidebar_menu_item.active_routes) do += nav_link(**sidebar_menu_item.active_routes, html_options: sidebar_menu_item.nav_link_html_options) do = link_to sidebar_menu_item.link, **sidebar_menu_item.container_html_options, data: { qa_selector: 'sidebar_menu_item_link', qa_menu_item: sidebar_menu_item.title } do %span = sidebar_menu_item.title diff --git a/app/workers/concerns/worker_attributes.rb b/app/workers/concerns/worker_attributes.rb index 102f1ca144d..096be808787 100644 --- a/app/workers/concerns/worker_attributes.rb +++ b/app/workers/concerns/worker_attributes.rb @@ -186,6 +186,12 @@ module WorkerAttributes class_attributes[:deduplication_options] || {} end + def deduplication_enabled? + return true unless get_deduplication_options[:feature_flag] + + Feature.enabled?(get_deduplication_options[:feature_flag], default_enabled: :yaml) + end + def big_payload! set_class_attribute(:big_payload, true) end diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb index 6b9f90ce1fc..b3b3d6e7554 100644 --- a/app/workers/stuck_ci_jobs_worker.rb +++ b/app/workers/stuck_ci_jobs_worker.rb @@ -15,22 +15,46 @@ class StuckCiJobsWorker # rubocop:disable Scalability/IdempotentWorker BUILD_PENDING_OUTDATED_TIMEOUT = 1.day BUILD_SCHEDULED_OUTDATED_TIMEOUT = 1.hour BUILD_PENDING_STUCK_TIMEOUT = 1.hour + BUILD_LOOKBACK = 5.days def perform return unless try_obtain_lease Gitlab::AppLogger.info "#{self.class}: Cleaning stuck builds" - drop :running, BUILD_RUNNING_OUTDATED_TIMEOUT, 'ci_builds.updated_at < ?', :stuck_or_timeout_failure - drop :pending, BUILD_PENDING_OUTDATED_TIMEOUT, 'ci_builds.updated_at < ?', :stuck_or_timeout_failure - drop :scheduled, BUILD_SCHEDULED_OUTDATED_TIMEOUT, 'scheduled_at IS NOT NULL AND scheduled_at < ?', :stale_schedule - drop_stuck :pending, BUILD_PENDING_STUCK_TIMEOUT, 'ci_builds.updated_at < ?', :stuck_or_timeout_failure + drop(running_timed_out_builds, failure_reason: :stuck_or_timeout_failure) + + drop( + Ci::Build.pending.updated_before(lookback: BUILD_LOOKBACK.ago, timeout: BUILD_PENDING_OUTDATED_TIMEOUT.ago), + failure_reason: :stuck_or_timeout_failure + ) + + drop(scheduled_timed_out_builds, failure_reason: :stale_schedule) + + drop_stuck( + Ci::Build.pending.updated_before(lookback: BUILD_LOOKBACK.ago, timeout: BUILD_PENDING_STUCK_TIMEOUT.ago), + failure_reason: :stuck_or_timeout_failure + ) remove_lease end private + def scheduled_timed_out_builds + Ci::Build.where(status: :scheduled).where( # rubocop: disable CodeReuse/ActiveRecord + 'ci_builds.scheduled_at IS NOT NULL AND ci_builds.scheduled_at < ?', + BUILD_SCHEDULED_OUTDATED_TIMEOUT.ago + ) + end + + def running_timed_out_builds + Ci::Build.running.where( # rubocop: disable CodeReuse/ActiveRecord + 'ci_builds.updated_at < ?', + BUILD_RUNNING_OUTDATED_TIMEOUT.ago + ) + end + def try_obtain_lease @uuid = Gitlab::ExclusiveLease.new(EXCLUSIVE_LEASE_KEY, timeout: 30.minutes).try_obtain end @@ -39,28 +63,27 @@ class StuckCiJobsWorker # rubocop:disable Scalability/IdempotentWorker Gitlab::ExclusiveLease.cancel(EXCLUSIVE_LEASE_KEY, @uuid) end - def drop(status, timeout, condition, reason) - search(status, timeout, condition) do |build| - drop_build :outdated, build, status, timeout, reason + def drop(builds, failure_reason:) + fetch(builds) do |build| + drop_build :outdated, build, failure_reason end end - def drop_stuck(status, timeout, condition, reason) - search(status, timeout, condition) do |build| + def drop_stuck(builds, failure_reason:) + fetch(builds) do |build| break unless build.stuck? - drop_build :stuck, build, status, timeout, reason + drop_build :stuck, build, failure_reason end end # rubocop: disable CodeReuse/ActiveRecord - def search(status, timeout, condition) + def fetch(builds) loop do - jobs = Ci::Build.where(status: status) - .where(condition, timeout.ago) - .includes(:tags, :runner, project: [:namespace, :route]) + jobs = builds.includes(:tags, :runner, project: [:namespace, :route]) .limit(100) .to_a + break if jobs.empty? jobs.each do |job| @@ -70,8 +93,8 @@ class StuckCiJobsWorker # rubocop:disable Scalability/IdempotentWorker end # rubocop: enable CodeReuse/ActiveRecord - def drop_build(type, build, status, timeout, reason) - Gitlab::AppLogger.info "#{self.class}: Dropping #{type} build #{build.id} for runner #{build.runner_id} (status: #{status}, timeout: #{timeout}, reason: #{reason})" + def drop_build(type, build, reason) + Gitlab::AppLogger.info "#{self.class}: Dropping #{type} build #{build.id} for runner #{build.runner_id} (status: #{build.status}, failure_reason: #{reason})" Gitlab::OptimisticLocking.retry_lock(build, 3, name: 'stuck_ci_jobs_worker_drop_build') do |b| b.drop(reason) end diff --git a/config/metrics/license/20210204124827_hostname.yml b/config/metrics/license/20210204124827_hostname.yml index 953239eff7a..40968500e13 100644 --- a/config/metrics/license/20210204124827_hostname.yml +++ b/config/metrics/license/20210204124827_hostname.yml @@ -17,4 +17,3 @@ tier: - free - premium - ultimate -skip_validation: true diff --git a/doc/administration/auth/ldap/index.md b/doc/administration/auth/ldap/index.md index fcedf8fffc5..1e2b5bfd942 100644 --- a/doc/administration/auth/ldap/index.md +++ b/doc/administration/auth/ldap/index.md @@ -653,6 +653,8 @@ NOTE: Administrators are not synced unless `group_base` is also specified alongside `admin_group`. Also, only specify the CN of the `admin_group`, as opposed to the full DN. +Additionally, note that if an LDAP user has an `admin` role, but is not a member of the `admin_group` +group, GitLab revokes their `admin` role when syncing. **Omnibus configuration** diff --git a/doc/administration/packages/container_registry.md b/doc/administration/packages/container_registry.md index ec90ceede31..344e032a780 100644 --- a/doc/administration/packages/container_registry.md +++ b/doc/administration/packages/container_registry.md @@ -1293,6 +1293,88 @@ curl "localhost:5001/debug/health" curl "localhost:5001/debug/vars" ``` +### Access old schema v1 Docker images + +Support for the [Docker registry v1 API](https://www.docker.com/blog/registry-v1-api-deprecation/), +including [schema V1 image manifests](https://docs.docker.com/registry/spec/manifest-v2-1/), +was: + +- [Deprecated in GitLab 13.7](https://about.gitlab.com/releases/2020/12/22/gitlab-13-7-released/#deprecate-pulls-that-use-v1-of-the-docker-registry-api) +- [Removed in GitLab 13.9](https://about.gitlab.com/releases/2021/02/22/gitlab-13-9-released/#deprecate-pulls-that-use-v1-of-the-docker-registry-api) + +It's no longer possible to push or pull v1 images from the GitLab Container Registry. + +If you had v1 images in the GitLab Container Registry, but you did not upgrade them (following the +[steps Docker recommends](https://docs.docker.com/registry/spec/deprecated-schema-v1/)) +ahead of the GitLab 13.9 upgrade, these images are no longer accessible. If you try to pull them, +this error appears: + +- `Error response from daemon: manifest invalid: Schema 1 manifest not supported` + +For Self-Managed GitLab instances, you can regain access to these images by temporarily downgrading +the GitLab Container Registry to a version lower than `v3.0.0-gitlab`. Follow these steps to regain +access to these images: + +1. Downgrade the Container Registry to [`v2.13.1-gitlab`](https://gitlab.com/gitlab-org/container-registry/-/releases/v2.13.1-gitlab). +1. Upgrade any v1 images. +1. Revert the Container Registry downgrade. + +There's no need to put the registry in read-only mode during the image upgrade process. Ensure that +you are not relying on any new feature introduced since `v3.0.0-gitlab`. Such features are +unavailable during the upgrade process. See the [complete registry changelog](https://gitlab.com/gitlab-org/container-registry/-/blob/master/CHANGELOG.md) +for more information. + +The following sections provide additional details about each installation method. + +#### Helm chart installations + +For Helm chart installations: + +1. Override the [`image.tag`](https://docs.gitlab.com/charts/charts/registry/#configuration) + configuration parameter with `v2.13.1-gitlab`. +1. Restart. +1. Performing the [images upgrade](#images-upgrade)) steps. +1. Revert the `image.tag` parameter to the previous value. + +No other registry configuration changes are required. + +#### Omnibus installations + +For Omnibus installations: + +1. Temporarily replace the registry binary that ships with GitLab 13.9+ for one prior to + `v3.0.0-gitlab`. To do so, pull a previous version of the Docker image for the GitLab Container + Registry, such as `v2.13.1-gitlab`. You can then grab the `registry` binary from within this + image, located at `/bin/registry`: + + ```shell + id=$(docker create registry.gitlab.com/gitlab-org/build/cng/gitlab-container-registry:v2.13.1-gitlab) + docker cp $id:/bin/registry registry-2.13.1-gitlab + docker rm $id + ``` + +1. Replace the binary embedded in the Omnibus install, located at + `/opt/gitlab/embedded/bin/registry`, with `registry-2.13.1-gitlab`. Make sure to start by backing + up the original binary embedded in Omnibus, and restore it after performing the + [image upgrade](#images-upgrade)) steps. You should [stop](https://docs.gitlab.com/omnibus/maintenance/#starting-and-stopping) + the registry service before replacing its binary and start it right after. No registry + configuration changes are required. + +#### Source installations + +For source installations, locate your `registry` binary and temporarily replace it with the one +obtained from `v3.0.0-gitlab`, as explained for [Omnibus installations](#omnibus-installations). +Make sure to start by backing up the original registry binary, and restore it after performing the +[images upgrade](#images-upgrade)) +steps. + +#### Images upgrade + +Follow the [steps that Docker recommends to upgrade v1 images](https://docs.docker.com/registry/spec/deprecated-schema-v1/). +The most straightforward option is to pull those images and push them once again to the registry, +using a Docker client version above v1.12. Docker converts images automatically before pushing them +to the registry. Once done, all your v1 images should now be available as v2 images. + ### Advanced Troubleshooting We use a concrete example to illustrate how to diff --git a/doc/development/usage_ping/dictionary.md b/doc/development/usage_ping/dictionary.md index fa2b933c5ab..2002c70e7d0 100644 --- a/doc/development/usage_ping/dictionary.md +++ b/doc/development/usage_ping/dictionary.md @@ -7712,7 +7712,7 @@ Group: `group::product intelligence` Status: `data_available` -Tiers: +Tiers: `premium`, `ultimate` ### `redis_hll_counters.analytics.analytics_total_unique_counts_monthly` diff --git a/doc/user/group/import/index.md b/doc/user/group/import/index.md index c63d43983a1..8bf995a4fd9 100644 --- a/doc/user/group/import/index.md +++ b/doc/user/group/import/index.md @@ -117,7 +117,8 @@ on an existing group's page. ### Selecting which groups to import -After you have authorized access to GitLab instance, you are redirected to the GitLab Group Migration importer page and your remote GitLab groups are listed. +After you have authorized access to the GitLab instance, you are redirected to the GitLab Group +Migration importer page. Your remote GitLab groups, which you have Owner access to, are listed. 1. By default, the proposed group namespaces match the names as they exist in remote instance, but based on your permissions, you can choose to edit these names before you proceed to import any of them. diff --git a/doc/user/group/value_stream_analytics/index.md b/doc/user/group/value_stream_analytics/index.md index 01c6c3dcf37..c1dd363c313 100644 --- a/doc/user/group/value_stream_analytics/index.md +++ b/doc/user/group/value_stream_analytics/index.md @@ -65,7 +65,7 @@ To filter results: > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13216) in GitLab 12.4. -GitLab provides the ability to filter analytics based on a date range. To filter results: +GitLab provides the ability to filter analytics based on a date range. Data is shown for workflow items created during the selected date range. To filter results: 1. Select a group. 1. Optionally select a project. diff --git a/doc/user/packages/container_registry/index.md b/doc/user/packages/container_registry/index.md index 5dc425f4911..9d65c5d37ad 100644 --- a/doc/user/packages/container_registry/index.md +++ b/doc/user/packages/container_registry/index.md @@ -512,6 +512,17 @@ On GitLab.com, the execution time for the cleanup policy is limited, and some of the Container Registry after the policy runs. The next time the policy runs, the remaining tags are included, so it may take multiple runs for all tags to be deleted. +WARNING: +GitLab self-managed installs support for third-party container registries that comply with the +[Docker Registry HTTP API V2](https://docs.docker.com/registry/spec/api/) +specification. However, this specification does not include a tag delete operation. Therefore, when +interacting with third-party container registries, GitLab uses a workaround to delete tags. See the +[related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/15737) +for more information. Due to possible implementation variations, this workaround is not guaranteed +to work with all third-party registries in the same predictable way. If you use the GitLab Container +Registry, this workaround is not required because we implemented a special tag delete operation. In +this case, you can expect cleanup policies to be consistent and predictable. + ### Create a cleanup policy You can create a cleanup policy in [the API](#use-the-cleanup-policy-api) or the UI. diff --git a/doc/user/packages/npm_registry/index.md b/doc/user/packages/npm_registry/index.md index 54a899bb949..735873be237 100644 --- a/doc/user/packages/npm_registry/index.md +++ b/doc/user/packages/npm_registry/index.md @@ -110,7 +110,8 @@ To authenticate, use one of the following: - It's not recommended, but you can use [OAuth tokens](../../../api/oauth2.md#resource-owner-password-credentials-flow). Standard OAuth tokens cannot authenticate to the GitLab npm Registry. You must use a personal access token with OAuth headers. - A [CI job token](#authenticate-with-a-ci-job-token). -- Your npm package name must be in the format of [@scope/package-name](#package-naming-convention). It must match exactly, including the case. +- Your npm package name must be in the format of [`@scope/package-name`](#package-naming-convention). + It must match exactly, including the case. ### Authenticate with a personal access token or deploy token @@ -282,7 +283,7 @@ Prerequisites: - [Authenticate](#authenticate-to-the-package-registry) to the Package Registry. - Set a [project-level npm endpoint](#use-the-gitlab-endpoint-for-npm-packages). -- Your npm package name must be in the format of [@scope/package-name](#package-naming-convention). +- Your npm package name must be in the format of [`@scope/package-name`](#package-naming-convention). It must match exactly, including the case. This is different than the npm naming convention, but it is required to work with the GitLab Package Registry. @@ -532,7 +533,7 @@ If you get this error, ensure that: ### `npm publish` returns `npm ERR! 400 Bad Request` If you get this error, your package name may not meet the -[@scope/package-name package naming convention](#package-naming-convention). +[`@scope/package-name` package naming convention](#package-naming-convention). Ensure the name meets the convention exactly, including the case. Then try to publish again. diff --git a/lib/gitlab/ci/trace.rb b/lib/gitlab/ci/trace.rb index c4757edf74e..84eb860a168 100644 --- a/lib/gitlab/ci/trace.rb +++ b/lib/gitlab/ci/trace.rb @@ -14,6 +14,8 @@ module Gitlab UPDATE_FREQUENCY_DEFAULT = 60.seconds UPDATE_FREQUENCY_WHEN_BEING_WATCHED = 3.seconds + LOAD_BALANCING_STICKING_NAMESPACE = 'ci/build/trace' + ArchiveError = Class.new(StandardError) AlreadyArchivedError = Class.new(StandardError) LockedError = Class.new(StandardError) @@ -296,25 +298,31 @@ module Gitlab read_trace_artifact(job) { job.job_artifacts_trace } end - ## - # Overridden in EE - # - def destroy_stream(job) + def destroy_stream(build) + if consistent_archived_trace?(build) + ::Gitlab::Database::LoadBalancing::Sticking + .stick(LOAD_BALANCING_STICKING_NAMESPACE, build.id) + end + yield end - ## - # Overriden in EE - # - def read_trace_artifact(job) + def read_trace_artifact(build) + if consistent_archived_trace?(build) + ::Gitlab::Database::LoadBalancing::Sticking + .unstick_or_continue_sticking(LOAD_BALANCING_STICKING_NAMESPACE, build.id) + end + yield end + def consistent_archived_trace?(build) + ::Feature.enabled?(:gitlab_ci_archived_trace_consistent_reads, build.project, default_enabled: false) + end + def being_watched_cache_key "gitlab:ci:trace:#{job.id}:watched" end end end end - -::Gitlab::Ci::Trace.prepend_mod_with('Gitlab::Ci::Trace') diff --git a/lib/gitlab/error_tracking.rb b/lib/gitlab/error_tracking.rb index 635e84d799f..38ac5d9af74 100644 --- a/lib/gitlab/error_tracking.rb +++ b/lib/gitlab/error_tracking.rb @@ -19,21 +19,22 @@ module Gitlab PROCESSORS = [ ::Gitlab::ErrorTracking::Processor::SidekiqProcessor, ::Gitlab::ErrorTracking::Processor::GrpcErrorProcessor, - ::Gitlab::ErrorTracking::Processor::ContextPayloadProcessor, - # IMPORTANT: this processor must stay at the bottom, right before - # sending the event to Sentry. - ::Gitlab::ErrorTracking::Processor::SanitizerProcessor + ::Gitlab::ErrorTracking::Processor::ContextPayloadProcessor ].freeze class << self def configure - Sentry.init do |config| + Raven.configure do |config| config.dsn = sentry_dsn config.release = Gitlab.revision - config.environment = Gitlab.config.sentry.environment + config.current_environment = Gitlab.config.sentry.environment + + # Sanitize fields based on those sanitized from Rails. + config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s) + + # Sanitize authentication headers + config.sanitize_http_headers = %w[Authorization Private-Token] config.before_send = method(:before_send) - config.background_worker_threads = 0 - config.send_default_pii = true yield config if block_given? end @@ -107,11 +108,8 @@ module Gitlab def process_exception(exception, sentry: false, logging: true, extra:) context_payload = Gitlab::ErrorTracking::ContextPayloadGenerator.generate(exception, extra) - # There is a possibility that this method is called before Sentry is - # configured. Since Sentry 4.0, some methods of Sentry are forwarded to - # to `nil`, hence we have to check the client as well. - if sentry && ::Sentry.get_current_client && ::Sentry.configuration.dsn - ::Sentry.capture_exception(exception, **context_payload) + if sentry && Raven.configuration.server + Raven.capture_exception(exception, **context_payload) end if logging diff --git a/lib/gitlab/error_tracking/log_formatter.rb b/lib/gitlab/error_tracking/log_formatter.rb index 92ef4d957f3..d004c4e20bb 100644 --- a/lib/gitlab/error_tracking/log_formatter.rb +++ b/lib/gitlab/error_tracking/log_formatter.rb @@ -3,7 +3,7 @@ module Gitlab module ErrorTracking class LogFormatter - # Note: all the accesses to Sentry's contexts here are to keep the + # Note: all the accesses to Raven's contexts here are to keep the # backward-compatibility to Sentry's built-in integrations. In future, # they can be removed. def generate_log(exception, context_payload) @@ -20,27 +20,21 @@ module Gitlab private def append_user_to_log!(payload, context_payload) - return if current_scope.blank? - - user_context = current_scope.user.merge(context_payload[:user]) + user_context = Raven.context.user.merge(context_payload[:user]) user_context.each do |key, value| payload["user.#{key}"] = value end end def append_tags_to_log!(payload, context_payload) - return if current_scope.blank? - - tags_context = current_scope.tags.merge(context_payload[:tags]) + tags_context = Raven.context.tags.merge(context_payload[:tags]) tags_context.each do |key, value| payload["tags.#{key}"] = value end end def append_extra_to_log!(payload, context_payload) - return if current_scope.blank? - - extra = current_scope.extra.merge(context_payload[:extra]) + extra = Raven.context.extra.merge(context_payload[:extra]) extra = extra.except(:server) # The extra value for sidekiq is a hash whose keys are strings. @@ -56,10 +50,6 @@ module Gitlab payload["extra.#{key}"] = value end end - - def current_scope - Sentry.get_current_scope - end end end end diff --git a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb index 0c2f1b2be67..e2a9192806f 100644 --- a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb +++ b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb @@ -17,7 +17,8 @@ module Gitlab # Sentry can report multiple exceptions in an event. Sanitize # only the first one since that's what is used for grouping. def process_first_exception_value(event) - exceptions = event.exception&.instance_variable_get(:@values) + # Better in new version, will be event.exception.values + exceptions = event.instance_variable_get(:@interfaces)[:exception]&.values return unless exceptions.is_a?(Array) @@ -32,7 +33,9 @@ module Gitlab message, debug_str = split_debug_error_string(raw_message) - exception.instance_variable_set(:@value, message) if message + # Worse in new version, no setter! Have to poke at the + # instance variable + exception.value = message if message event.extra[:grpc_debug_error_string] = debug_str if debug_str end @@ -63,7 +66,7 @@ module Gitlab def valid_exception?(exception) case exception - when Sentry::SingleExceptionInterface + when Raven::SingleExceptionInterface exception&.value else false diff --git a/lib/gitlab/error_tracking/processor/sanitizer_processor.rb b/lib/gitlab/error_tracking/processor/sanitizer_processor.rb deleted file mode 100644 index 32d441fcdef..00000000000 --- a/lib/gitlab/error_tracking/processor/sanitizer_processor.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module ErrorTracking - module Processor - module SanitizerProcessor - SANITIZED_HTTP_HEADERS = %w[Authorization Private-Token].freeze - SANITIZED_ATTRIBUTES = %i[user contexts extra tags].freeze - - # This processor removes sensitive fields or headers from the event - # before sending. Sentry versions above 4.0 don't support - # sanitized_fields and sanitized_http_headers anymore. The official - # document recommends using before_send instead. - # - # For more information, please visit: - # https://docs.sentry.io/platforms/ruby/guides/rails/configuration/filtering/#using-beforesend - def self.call(event) - if event.request.present? && event.request.headers.is_a?(Hash) - header_filter = ActiveSupport::ParameterFilter.new(SANITIZED_HTTP_HEADERS) - event.request.headers = header_filter.filter(event.request.headers) - end - - attribute_filter = ActiveSupport::ParameterFilter.new(Rails.application.config.filter_parameters) - SANITIZED_ATTRIBUTES.each do |attribute| - event.send("#{attribute}=", attribute_filter.filter(event.send(attribute))) # rubocop:disable GitlabSecurity/PublicSend - end - - event - end - end - end - end -end diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb index b456dcdb666..4cf540ce3b8 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb @@ -19,6 +19,7 @@ module Gitlab class DuplicateJob DUPLICATE_KEY_TTL = 6.hours DEFAULT_STRATEGY = :until_executing + STRATEGY_NONE = :none attr_reader :existing_jid @@ -102,6 +103,7 @@ module Gitlab def strategy return DEFAULT_STRATEGY unless worker_klass return DEFAULT_STRATEGY unless worker_klass.respond_to?(:idempotent?) + return STRATEGY_NONE unless worker_klass.deduplication_enabled? worker_klass.get_deduplicate_strategy end diff --git a/lib/peek/views/active_record.rb b/lib/peek/views/active_record.rb index 774e4768597..8e1200338c2 100644 --- a/lib/peek/views/active_record.rb +++ b/lib/peek/views/active_record.rb @@ -43,6 +43,11 @@ module Peek count[item[:transaction]] ||= 0 count[item[:transaction]] += 1 end + + if ::Gitlab::Database::LoadBalancing.enable? + count[item[:db_role]] ||= 0 + count[item[:db_role]] += 1 + end end def setup_subscribers @@ -60,11 +65,19 @@ module Peek sql: data[:sql].strip, backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller), cached: data[:cached] ? 'Cached' : '', - transaction: data[:connection].transaction_open? ? 'In a transaction' : '' + transaction: data[:connection].transaction_open? ? 'In a transaction' : '', + db_role: db_role(data) } end + + def db_role(data) + return unless ::Gitlab::Database::LoadBalancing.enable? + + role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection]) || + ::Gitlab::Database::LoadBalancing::ROLE_UNKNOWN + + role.to_s.capitalize + end end end end - -Peek::Views::ActiveRecord.prepend_mod_with('Peek::Views::ActiveRecord') diff --git a/lib/sidebars/concerns/container_with_html_options.rb b/lib/sidebars/concerns/container_with_html_options.rb index 873cb5b0de9..79dddd309b5 100644 --- a/lib/sidebars/concerns/container_with_html_options.rb +++ b/lib/sidebars/concerns/container_with_html_options.rb @@ -38,6 +38,16 @@ module Sidebars # in the helper method that sets the active class # on each element. def nav_link_html_options + { + data: { + track_label: self.class.name.demodulize.underscore + } + }.deep_merge(extra_nav_link_html_options) + end + + # Classes should mostly override this method + # and not `nav_link_html_options`. + def extra_nav_link_html_options {} end diff --git a/lib/sidebars/menu_item.rb b/lib/sidebars/menu_item.rb index b0a12e769dc..1375f9fffca 100644 --- a/lib/sidebars/menu_item.rb +++ b/lib/sidebars/menu_item.rb @@ -22,5 +22,13 @@ module Sidebars def render? true end + + def nav_link_html_options + { + data: { + track_label: item_id + } + } + end end end diff --git a/lib/sidebars/projects/menus/learn_gitlab_menu.rb b/lib/sidebars/projects/menus/learn_gitlab_menu.rb index e3fcd8f25d5..f29f4a6eed6 100644 --- a/lib/sidebars/projects/menus/learn_gitlab_menu.rb +++ b/lib/sidebars/projects/menus/learn_gitlab_menu.rb @@ -35,14 +35,13 @@ module Sidebars end end - override :extra_container_html_options - def nav_link_html_options + override :extra_nav_link_html_options + def extra_nav_link_html_options { class: 'home', data: { - track_action: 'click_menu', - track_property: context.learn_gitlab_experiment_tracking_category, - track_label: 'learn_gitlab' + track_label: 'learn_gitlab', + track_property: context.learn_gitlab_experiment_tracking_category } } end diff --git a/lib/sidebars/projects/menus/project_information_menu.rb b/lib/sidebars/projects/menus/project_information_menu.rb index 898fd2fb67e..c148e7cf931 100644 --- a/lib/sidebars/projects/menus/project_information_menu.rb +++ b/lib/sidebars/projects/menus/project_information_menu.rb @@ -29,8 +29,8 @@ module Sidebars end end - override :nav_link_html_options - def nav_link_html_options + override :extra_nav_link_html_options + def extra_nav_link_html_options { class: 'home' } end diff --git a/lib/sidebars/projects/menus/scope_menu.rb b/lib/sidebars/projects/menus/scope_menu.rb index a86d8a467fc..1cd0218d4ac 100644 --- a/lib/sidebars/projects/menus/scope_menu.rb +++ b/lib/sidebars/projects/menus/scope_menu.rb @@ -28,8 +28,8 @@ module Sidebars } end - override :nav_link_html_options - def nav_link_html_options + override :extra_nav_link_html_options + def extra_nav_link_html_options return {} if Feature.disabled?(:sidebar_refactor, context.current_user, default_enabled: :yaml) { class: 'context-header' } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2b5878eb3ab..9bc0d4a1463 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10437,9 +10437,6 @@ msgstr "" msgid "Date range" msgstr "" -msgid "Date range cannot exceed %{maxDateRange} days." -msgstr "" - msgid "Date range must be shorter than %{max_range} days." msgstr "" @@ -26792,9 +26789,6 @@ msgstr "" msgid "Protocol" msgstr "" -msgid "Provide feedback" -msgstr "" - msgid "Provider" msgstr "" @@ -30192,6 +30186,9 @@ msgstr "" msgid "Showing all issues" msgstr "" +msgid "Showing data for workflow items created in this date range. Date range cannot exceed %{maxDateRange} days." +msgstr "" + msgid "Showing graphs based on events of the last %{timerange} days." msgstr "" @@ -33000,9 +32997,6 @@ msgstr "" msgid "There was a problem communicating with your device." msgstr "" -msgid "There was a problem dismissing this notification." -msgstr "" - msgid "There was a problem fetching branches." msgstr "" @@ -36139,9 +36133,6 @@ msgstr "" msgid "View job" msgstr "" -msgid "View job dependencies in the pipeline graph!" -msgstr "" - msgid "View job log" msgstr "" @@ -37401,9 +37392,6 @@ msgstr "" msgid "You can now export your security dashboard to a CSV report." msgstr "" -msgid "You can now group jobs in the pipeline graph based on which jobs are configured to run first, if you use the %{codeStart}needs:%{codeEnd} keyword to establish job dependencies in your CI/CD pipelines. %{linkStart}Learn how to speed up your pipeline with needs.%{linkEnd}" -msgstr "" - msgid "You can now submit a merge request to get this change into the original branch." msgstr "" diff --git a/spec/controllers/import/bulk_imports_controller_spec.rb b/spec/controllers/import/bulk_imports_controller_spec.rb index b450318f6f7..12aa1d89ecc 100644 --- a/spec/controllers/import/bulk_imports_controller_spec.rb +++ b/spec/controllers/import/bulk_imports_controller_spec.rb @@ -73,7 +73,7 @@ RSpec.describe Import::BulkImportsController do let(:client_params) do { top_level_only: true, - min_access_level: Gitlab::Access::MAINTAINER + min_access_level: Gitlab::Access::OWNER } end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 989f941caea..4fcb63ac616 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -69,6 +69,20 @@ RSpec.describe Projects::MergeRequests::DiffsController do end end + shared_examples 'show the right diff files with previous diff_id' do + context 'with previous diff_id' do + let!(:merge_request_diff_1) { merge_request.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff_2) { merge_request.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e', diff_type: :merge_head) } + + subject { go(diff_id: merge_request_diff_1.id, diff_head: true) } + + it 'shows the right diff files' do + subject + expect(json_response["diff_files"].size).to eq(merge_request_diff_1.files_count) + end + end + end + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } @@ -142,6 +156,8 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like '404 for unexistent diffable' + it_behaves_like 'show the right diff files with previous diff_id' + context 'when not authorized' do let(:another_user) { create(:user) } @@ -480,6 +496,8 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like '404 for unexistent diffable' + it_behaves_like 'show the right diff files with previous diff_id' + context 'when not authorized' do let(:other_user) { create(:user) } @@ -499,7 +517,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'serializes diffs with expected arguments' do let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } - let(:expected_options) { collection_arguments(current_page: 1, total_pages: 1) } + let(:expected_options) { collection_arguments(current_page: 1, total_pages: 1).merge(merge_ref_head_diff: false) } end it_behaves_like 'successful request' @@ -522,7 +540,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'serializes diffs with expected arguments' do let(:collection) { Gitlab::Diff::FileCollection::Compare } - let(:expected_options) { collection_arguments } + let(:expected_options) { collection_arguments.merge(merge_ref_head_diff: false) } end it_behaves_like 'successful request' diff --git a/spec/features/groups/import_export/connect_instance_spec.rb b/spec/features/groups/import_export/connect_instance_spec.rb index a281180c54f..563c8f429f8 100644 --- a/spec/features/groups/import_export/connect_instance_spec.rb +++ b/spec/features/groups/import_export/connect_instance_spec.rb @@ -24,7 +24,7 @@ RSpec.describe 'Import/Export - Connect to another instance', :js do pat = 'demo-pat' stub_path = 'stub-group' total = 37 - stub_request(:get, "%{url}/api/v4/groups?page=1&per_page=20&top_level_only=true&min_access_level=40&search=" % { url: source_url }).to_return( + stub_request(:get, "%{url}/api/v4/groups?page=1&per_page=20&top_level_only=true&min_access_level=50&search=" % { url: source_url }).to_return( body: [{ id: 2595438, web_url: 'https://gitlab.com/groups/auto-breakfast', diff --git a/spec/features/markdown/mermaid_spec.rb b/spec/features/markdown/mermaid_spec.rb index 207678e07c3..75c1d03b638 100644 --- a/spec/features/markdown/mermaid_spec.rb +++ b/spec/features/markdown/mermaid_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe 'Mermaid rendering', :js do + let_it_be(:project) { create(:project, :public) } + it 'renders Mermaid diagrams correctly' do description = <<~MERMAID ```mermaid @@ -14,7 +16,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -36,7 +37,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -44,10 +44,33 @@ RSpec.describe 'Mermaid rendering', :js do wait_for_requests wait_for_mermaid - expected = '<text style=""><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>' + # From https://github.com/mermaid-js/mermaid/blob/d3f8f03a7d03a052e1fe0251d5a6d8d1f48d67ee/src/dagre-wrapper/createLabel.js#L79-L82 + expected = %(<div xmlns="http://www.w3.org/1999/xhtml" style="display: inline-block; white-space: nowrap;">Line 1<br>Line 2</div>) expect(page.html.scan(expected).count).to be(4) end + it 'does not allow XSS in HTML labels' do + description = <<~MERMAID + ```mermaid + graph LR; + A-->CLICK_HERE_AND_GET_BONUS; + click A alert "aaa" + click CLICK_HERE_AND_GET_BONUS "javascript:alert%28%64%6f%63%75%6d%65%6e%74%2e%64%6f%6d%61%69%6e%29" "Here is the XSS" + ``` + MERMAID + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + wait_for_requests + wait_for_mermaid + + # From https://github.com/mermaid-js/mermaid/blob/d3f8f03a7d03a052e1fe0251d5a6d8d1f48d67ee/src/dagre-wrapper/createLabel.js#L79-L82 + expected = %(<div xmlns="http://www.w3.org/1999/xhtml" style="display: inline-block; white-space: nowrap;">CLICK_HERE_AND_GET_BONUS</div>) + expect(page.html).to include(expected) + end + it 'renders only 2 Mermaid blocks and', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/234081' do description = <<~MERMAID ```mermaid @@ -64,7 +87,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -94,7 +116,6 @@ RSpec.describe 'Mermaid rendering', :js do </details> MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -108,7 +129,37 @@ RSpec.describe 'Mermaid rendering', :js do expect(svg[:style]).to match(/max-width/) expect(svg[:width].to_i).to eq(100) - expect(svg[:height].to_i).to be_within(5).of(220) + expect(svg[:height].to_i).to be_within(5).of(236) + end + end + + it 'renders V2 state diagrams' do + description = <<~MERMAID + ```mermaid + stateDiagram-v2 + [*] --> Idle + Idle --> Active : CONTINUE + state Active { + [*] --> Run + Run--> Stop: CONTINUE + Stop--> Run: CONTINUE + + Run: Run + Run: entry/start + Run: check + } + ``` + MERMAID + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + wait_for_requests + wait_for_mermaid + + page.within('.description') do + expect(page).to have_selector('svg') end end @@ -123,7 +174,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -144,7 +194,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -183,8 +232,6 @@ RSpec.describe 'Mermaid rendering', :js do description *= 51 - project = create(:project, :public) - issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) diff --git a/spec/features/security/project/snippet/private_access_spec.rb b/spec/features/security/project/snippet/private_access_spec.rb index 0f7ae06a6c5..0c06841399d 100644 --- a/spec/features/security/project/snippet/private_access_spec.rb +++ b/spec/features/security/project/snippet/private_access_spec.rb @@ -5,23 +5,25 @@ require 'spec_helper' RSpec.describe "Private Project Snippets Access" do include AccessMatchers - let(:project) { create(:project, :private) } - - let(:private_snippet) { create(:project_snippet, :private, project: project, author: project.owner) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:private_snippet) { create(:project_snippet, :private, project: project, author: project.owner) } describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { is_expected.to be_allowed_for(:admin) } it('is denied for admin when admin mode is disabled') { is_expected.to be_denied_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:maintainer).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_allowed_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } + + specify :aggregate_failures do + is_expected.to be_allowed_for(:owner).of(project) + is_expected.to be_allowed_for(:maintainer).of(project) + is_expected.to be_allowed_for(:developer).of(project) + is_expected.to be_allowed_for(:reporter).of(project) + is_expected.to be_allowed_for(:guest).of(project) + is_expected.to be_denied_for(:user) + is_expected.to be_denied_for(:external) + is_expected.to be_denied_for(:visitor) + end end describe "GET /:project_path/snippets/new" do @@ -29,14 +31,17 @@ RSpec.describe "Private Project Snippets Access" do it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { is_expected.to be_allowed_for(:admin) } it('is denied for admin when admin mode is disabled') { is_expected.to be_denied_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:maintainer).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_denied_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } + + specify :aggregate_failures do + is_expected.to be_allowed_for(:maintainer).of(project) + is_expected.to be_allowed_for(:owner).of(project) + is_expected.to be_allowed_for(:developer).of(project) + is_expected.to be_allowed_for(:reporter).of(project) + is_expected.to be_denied_for(:guest).of(project) + is_expected.to be_denied_for(:user) + is_expected.to be_denied_for(:external) + is_expected.to be_denied_for(:visitor) + end end describe "GET /:project_path/snippets/:id for a private snippet" do @@ -44,14 +49,17 @@ RSpec.describe "Private Project Snippets Access" do it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { is_expected.to be_allowed_for(:admin) } it('is denied for admin when admin mode is disabled') { is_expected.to be_denied_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:maintainer).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_allowed_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } + + specify :aggregate_failures do + is_expected.to be_allowed_for(:owner).of(project) + is_expected.to be_allowed_for(:maintainer).of(project) + is_expected.to be_allowed_for(:developer).of(project) + is_expected.to be_allowed_for(:reporter).of(project) + is_expected.to be_allowed_for(:guest).of(project) + is_expected.to be_denied_for(:user) + is_expected.to be_denied_for(:external) + is_expected.to be_denied_for(:visitor) + end end describe "GET /:project_path/snippets/:id/raw for a private snippet" do @@ -59,13 +67,16 @@ RSpec.describe "Private Project Snippets Access" do it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { is_expected.to be_allowed_for(:admin) } it('is denied for admin when admin mode is disabled') { is_expected.to be_denied_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:maintainer).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_allowed_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } + + specify :aggregate_failures do + is_expected.to be_allowed_for(:owner).of(project) + is_expected.to be_allowed_for(:maintainer).of(project) + is_expected.to be_allowed_for(:developer).of(project) + is_expected.to be_allowed_for(:reporter).of(project) + is_expected.to be_allowed_for(:guest).of(project) + is_expected.to be_denied_for(:user) + is_expected.to be_denied_for(:external) + is_expected.to be_denied_for(:visitor) + end end end diff --git a/spec/frontend/pages/shared/nav/sidebar_tracking_spec.js b/spec/frontend/pages/shared/nav/sidebar_tracking_spec.js new file mode 100644 index 00000000000..2c8eb8e459f --- /dev/null +++ b/spec/frontend/pages/shared/nav/sidebar_tracking_spec.js @@ -0,0 +1,160 @@ +import { setHTMLFixture } from 'helpers/fixtures'; +import { initSidebarTracking } from '~/pages/shared/nav/sidebar_tracking'; + +describe('~/pages/shared/nav/sidebar_tracking.js', () => { + beforeEach(() => { + setHTMLFixture(` + <aside class="nav-sidebar"> + <div class="nav-sidebar-inner-scroll"> + <ul class="sidebar-top-level-items"> + <li data-track-label="project_information_menu" class="home"> + <a aria-label="Project information" class="shortcuts-project-information has-sub-items" href=""> + <span class="nav-icon-container"> + <svg class="s16" data-testid="project-icon"> + <use xlink:href="/assets/icons-1b2dadc4c3d49797908ba67b8f10da5d63dd15d859bde28d66fb60bbb97a4dd5.svg#project"></use> + </svg> + </span> + <span class="nav-item-name">Project information</span> + </a> + <ul class="sidebar-sub-level-items"> + <li class="fly-out-top-item"> + <a aria-label="Project information" href="#"> + <strong class="fly-out-top-item-name">Project information</strong> + </a> + </li> + <li class="divider fly-out-top-item"></li> + <li data-track-label="activity" class=""> + <a aria-label="Activity" class="shortcuts-project-activity" href=#"> + <span>Activity</span> + </a> + </li> + <li data-track-label="labels" class=""> + <a aria-label="Labels" href="#"> + <span>Labels</span> + </a> + </li> + <li data-track-label="members" class=""> + <a aria-label="Members" href="#"> + <span>Members</span> + </a> + </li> + </ul> + </li> + </ul> + </div> + </aside> + `); + + initSidebarTracking(); + }); + + describe('sidebar is not collapsed', () => { + describe('menu is not expanded', () => { + it('sets the proper data tracking attributes when clicking on menu', () => { + const menu = document.querySelector('li[data-track-label="project_information_menu"]'); + const menuLink = menu.querySelector('a'); + + menu.classList.add('is-over', 'is-showing-fly-out'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu', + trackExtra: JSON.stringify({ + sidebar_display: 'Expanded', + menu_display: 'Fly out', + }), + }); + }); + + it('sets the proper data tracking attributes when clicking on submenu', () => { + const menu = document.querySelector('li[data-track-label="activity"]'); + const menuLink = menu.querySelector('a'); + const submenuList = document.querySelector('ul.sidebar-sub-level-items'); + + submenuList.classList.add('fly-out-list'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu_item', + trackExtra: JSON.stringify({ + sidebar_display: 'Expanded', + menu_display: 'Fly out', + }), + }); + }); + }); + + describe('menu is expanded', () => { + it('sets the proper data tracking attributes when clicking on menu', () => { + const menu = document.querySelector('li[data-track-label="project_information_menu"]'); + const menuLink = menu.querySelector('a'); + + menu.classList.add('active'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu', + trackExtra: JSON.stringify({ + sidebar_display: 'Expanded', + menu_display: 'Expanded', + }), + }); + }); + + it('sets the proper data tracking attributes when clicking on submenu', () => { + const menu = document.querySelector('li[data-track-label="activity"]'); + const menuLink = menu.querySelector('a'); + + menu.classList.add('active'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu_item', + trackExtra: JSON.stringify({ + sidebar_display: 'Expanded', + menu_display: 'Expanded', + }), + }); + }); + }); + }); + + describe('sidebar is collapsed', () => { + beforeEach(() => { + document.querySelector('aside.nav-sidebar').classList.add('js-sidebar-collapsed'); + }); + + it('sets the proper data tracking attributes when clicking on menu', () => { + const menu = document.querySelector('li[data-track-label="project_information_menu"]'); + const menuLink = menu.querySelector('a'); + + menu.classList.add('is-over', 'is-showing-fly-out'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu', + trackExtra: JSON.stringify({ + sidebar_display: 'Collapsed', + menu_display: 'Fly out', + }), + }); + }); + + it('sets the proper data tracking attributes when clicking on submenu', () => { + const menu = document.querySelector('li[data-track-label="activity"]'); + const menuLink = menu.querySelector('a'); + const submenuList = document.querySelector('ul.sidebar-sub-level-items'); + + submenuList.classList.add('fly-out-list'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu_item', + trackExtra: JSON.stringify({ + sidebar_display: 'Collapsed', + menu_display: 'Fly out', + }), + }); + }); + }); +}); diff --git a/spec/frontend/pipelines/graph_shared/__snapshots__/links_inner_spec.js.snap b/spec/frontend/pipelines/graph_shared/__snapshots__/links_inner_spec.js.snap index c67b91ae190..16c28791514 100644 --- a/spec/frontend/pipelines/graph_shared/__snapshots__/links_inner_spec.js.snap +++ b/spec/frontend/pipelines/graph_shared/__snapshots__/links_inner_spec.js.snap @@ -21,3 +21,10 @@ exports[`Links Inner component with one need matches snapshot and has expected p <path d=\\"M202,118L42,118C72,118,72,138,102,138\\" stroke-width=\\"2\\" class=\\"gl-fill-transparent gl-transition-duration-slow gl-transition-timing-function-ease gl-stroke-gray-200\\"></path> </svg> </div>" `; + +exports[`Links Inner component with same stage needs matches snapshot and has expected path 1`] = ` +"<div class=\\"gl-display-flex gl-relative\\" totalgroups=\\"10\\"><svg id=\\"link-svg\\" viewBox=\\"0,0,1019,445\\" width=\\"1019px\\" height=\\"445px\\" class=\\"gl-absolute gl-pointer-events-none\\"> + <path d=\\"M192,108L22,108C52,108,52,118,82,118\\" stroke-width=\\"2\\" class=\\"gl-fill-transparent gl-transition-duration-slow gl-transition-timing-function-ease gl-stroke-gray-200\\"></path> + <path d=\\"M202,118L32,118C62,118,62,128,92,128\\" stroke-width=\\"2\\" class=\\"gl-fill-transparent gl-transition-duration-slow gl-transition-timing-function-ease gl-stroke-gray-200\\"></path> + </svg> </div>" +`; diff --git a/spec/frontend/pipelines/graph_shared/links_inner_spec.js b/spec/frontend/pipelines/graph_shared/links_inner_spec.js index bb1f0965469..8f39c8c2405 100644 --- a/spec/frontend/pipelines/graph_shared/links_inner_spec.js +++ b/spec/frontend/pipelines/graph_shared/links_inner_spec.js @@ -10,6 +10,7 @@ import { pipelineData, pipelineDataWithNoNeeds, rootRect, + sameStageNeeds, } from '../pipeline_graph/mock_data'; describe('Links Inner component', () => { @@ -40,7 +41,7 @@ describe('Links Inner component', () => { // We create fixture so that each job has an empty div that represent // the JobPill in the DOM. Each `JobPill` would have different coordinates, - // so we increment their coordinates on each iteration to simulat different positions. + // so we increment their coordinates on each iteration to simulate different positions. const setFixtures = ({ stages }) => { const jobs = createJobsHash(stages); const arrayOfJobs = Object.keys(jobs); @@ -81,7 +82,6 @@ describe('Links Inner component', () => { afterEach(() => { jest.restoreAllMocks(); wrapper.destroy(); - wrapper = null; }); describe('basic SVG creation', () => { @@ -160,6 +160,25 @@ describe('Links Inner component', () => { }); }); + describe('with same stage needs', () => { + beforeEach(() => { + setFixtures(sameStageNeeds); + createComponent({ pipelineData: sameStageNeeds.stages }); + }); + + it('renders the correct number of links', () => { + expect(findAllLinksPath()).toHaveLength(2); + }); + + it('path does not contain NaN values', () => { + expect(wrapper.html()).not.toContain('NaN'); + }); + + it('matches snapshot and has expected path', () => { + expect(wrapper.html()).toMatchSnapshot(); + }); + }); + describe('with a large number of needs', () => { beforeEach(() => { setFixtures(largePipelineData); diff --git a/spec/frontend/pipelines/notification/pipeline_notification_spec.js b/spec/frontend/pipelines/notification/pipeline_notification_spec.js deleted file mode 100644 index 79aa337ba9d..00000000000 --- a/spec/frontend/pipelines/notification/pipeline_notification_spec.js +++ /dev/null @@ -1,79 +0,0 @@ -import { GlBanner } from '@gitlab/ui'; -import { createLocalVue, shallowMount } from '@vue/test-utils'; -import { nextTick } from 'vue'; -import VueApollo from 'vue-apollo'; -import createMockApollo from 'helpers/mock_apollo_helper'; -import PipelineNotification from '~/pipelines/components/notification/pipeline_notification.vue'; -import getUserCallouts from '~/pipelines/graphql/queries/get_user_callouts.query.graphql'; - -describe('Pipeline notification', () => { - const localVue = createLocalVue(); - - let wrapper; - const dagDocPath = 'my/dag/path'; - - const createWrapper = (apolloProvider) => { - return shallowMount(PipelineNotification, { - localVue, - provide: { - dagDocPath, - }, - apolloProvider, - }); - }; - - const createWrapperWithApollo = async ({ callouts = [], isLoading = false } = {}) => { - localVue.use(VueApollo); - - const mappedCallouts = callouts.map((callout) => { - return { featureName: callout, __typename: 'UserCallout' }; - }); - - const mockCalloutsResponse = { - data: { - currentUser: { - id: 45, - __typename: 'User', - callouts: { - id: 5, - __typename: 'UserCalloutConnection', - nodes: mappedCallouts, - }, - }, - }, - }; - const getUserCalloutsHandler = jest.fn().mockResolvedValue(mockCalloutsResponse); - const requestHandlers = [[getUserCallouts, getUserCalloutsHandler]]; - - const apolloWrapper = createWrapper(createMockApollo(requestHandlers)); - if (!isLoading) { - await nextTick(); - } - - return apolloWrapper; - }; - - const findBanner = () => wrapper.findComponent(GlBanner); - - afterEach(() => { - wrapper.destroy(); - }); - - it('shows the banner if the user has never seen it', async () => { - wrapper = await createWrapperWithApollo({ callouts: ['random'] }); - - expect(findBanner().exists()).toBe(true); - }); - - it('does not show the banner while the user callout query is loading', async () => { - wrapper = await createWrapperWithApollo({ callouts: ['random'], isLoading: true }); - - expect(findBanner().exists()).toBe(false); - }); - - it('does not show the banner if the user has previously dismissed it', async () => { - wrapper = await createWrapperWithApollo({ callouts: ['pipeline_needs_banner'.toUpperCase()] }); - - expect(findBanner().exists()).toBe(false); - }); -}); diff --git a/spec/frontend/pipelines/pipeline_graph/mock_data.js b/spec/frontend/pipelines/pipeline_graph/mock_data.js index a79917bfd48..db77e0a0573 100644 --- a/spec/frontend/pipelines/pipeline_graph/mock_data.js +++ b/spec/frontend/pipelines/pipeline_graph/mock_data.js @@ -162,6 +162,38 @@ export const parallelNeedData = { ], }; +export const sameStageNeeds = { + stages: [ + { + name: 'build', + groups: [ + { + name: 'build_1', + jobs: [{ script: 'echo hello', stage: 'build', name: 'build_1' }], + }, + ], + }, + { + name: 'build', + groups: [ + { + name: 'build_2', + jobs: [{ script: 'yarn test', stage: 'build', needs: ['build_1'] }], + }, + ], + }, + { + name: 'build', + groups: [ + { + name: 'build_3', + jobs: [{ script: 'yarn test', stage: 'build', needs: ['build_2'] }], + }, + ], + }, + ], +}; + export const largePipelineData = { stages: [ { diff --git a/spec/lib/gitlab/error_tracking/context_payload_generator_spec.rb b/spec/lib/gitlab/error_tracking/context_payload_generator_spec.rb index dffa7e6522e..0e72dd7ec5e 100644 --- a/spec/lib/gitlab/error_tracking/context_payload_generator_spec.rb +++ b/spec/lib/gitlab/error_tracking/context_payload_generator_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' require 'rspec-parameterized' RSpec.describe Gitlab::ErrorTracking::ContextPayloadGenerator do diff --git a/spec/lib/gitlab/error_tracking/log_formatter_spec.rb b/spec/lib/gitlab/error_tracking/log_formatter_spec.rb index c97aebf5cd3..188ccd000a1 100644 --- a/spec/lib/gitlab/error_tracking/log_formatter_spec.rb +++ b/spec/lib/gitlab/error_tracking/log_formatter_spec.rb @@ -27,9 +27,9 @@ RSpec.describe Gitlab::ErrorTracking::LogFormatter do end before do - Sentry.set_user(user_flag: 'flag') - Sentry.set_tags(shard: 'catchall') - Sentry.set_extras(some_info: 'info') + Raven.context.user[:user_flag] = 'flag' + Raven.context.tags[:shard] = 'catchall' + Raven.context.extra[:some_info] = 'info' allow(exception).to receive(:backtrace).and_return( [ @@ -40,7 +40,7 @@ RSpec.describe Gitlab::ErrorTracking::LogFormatter do end after do - ::Sentry.get_current_scope.clear + ::Raven::Context.clear! end it 'appends error-related log fields and filters sensitive Sidekiq arguments' do diff --git a/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb index 8b83d073e45..210829056c8 100644 --- a/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb @@ -4,14 +4,18 @@ require 'spec_helper' RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do describe '.call' do - let(:exception) { StandardError.new('Test exception') } - let(:event) { Sentry.get_current_client.event_from_exception(exception) } + let(:required_options) do + { + configuration: Raven.configuration, + context: Raven.context, + breadcrumbs: Raven.breadcrumbs + } + end + + let(:event) { Raven::Event.new(required_options.merge(payload)) } let(:result_hash) { described_class.call(event).to_hash } before do - Sentry.get_current_scope.update_from_options(**payload) - Sentry.get_current_scope.apply_to_event(event) - allow_next_instance_of(Gitlab::ErrorTracking::ContextPayloadGenerator) do |generator| allow(generator).to receive(:generate).and_return( user: { username: 'root' }, @@ -21,10 +25,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do end end - after do - Sentry.get_current_scope.clear - end - let(:payload) do { user: { ip_address: '127.0.0.1' }, diff --git a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb index 20fa5e2dacf..6076e525f06 100644 --- a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb @@ -4,17 +4,16 @@ require 'spec_helper' RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do describe '.call' do - let(:event) { Sentry.get_current_client.event_from_exception(exception) } - let(:result_hash) { described_class.call(event).to_hash } - - before do - Sentry.get_current_scope.update_from_options(**data) - Sentry.get_current_scope.apply_to_event(event) + let(:required_options) do + { + configuration: Raven.configuration, + context: Raven.context, + breadcrumbs: Raven.breadcrumbs + } end - after do - Sentry.get_current_scope.clear - end + let(:event) { Raven::Event.from_exception(exception, required_options.merge(data)) } + let(:result_hash) { described_class.call(event).to_hash } context 'when there is no GRPC exception' do let(:exception) { RuntimeError.new } @@ -57,7 +56,7 @@ RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do end it 'removes the debug error string and stores it as an extra field' do - expect(result_hash[:fingerprint]).to be_empty + expect(result_hash).not_to include(:fingerprint) expect(result_hash[:exception][:values].first) .to include(type: 'GRPC::DeadlineExceeded', value: '4:Deadline Exceeded.') diff --git a/spec/lib/gitlab/error_tracking/processor/sanitizer_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sanitizer_processor_spec.rb deleted file mode 100644 index 7e7d836f1d2..00000000000 --- a/spec/lib/gitlab/error_tracking/processor/sanitizer_processor_spec.rb +++ /dev/null @@ -1,96 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::ErrorTracking::Processor::SanitizerProcessor do - describe '.call' do - let(:event) { Sentry.get_current_client.event_from_exception(exception) } - let(:result_hash) { described_class.call(event).to_hash } - - before do - data.each do |key, value| - event.send("#{key}=", value) - end - end - - after do - Sentry.get_current_scope.clear - end - - context 'when event attributes contains sensitive information' do - let(:exception) { RuntimeError.new } - let(:data) do - { - contexts: { - jwt: 'abcdef', - controller: 'GraphController#execute' - }, - tags: { - variables: %w[some sensitive information'], - deep_hash: { - sharedSecret: 'secret123' - } - }, - user: { - email: 'a@a.com', - password: 'nobodyknows' - }, - extra: { - issue_url: 'http://gitlab.com/gitlab-org/gitlab-foss/-/issues/1', - my_token: '[FILTERED]', - another_token: '[FILTERED]' - } - } - end - - it 'filters sensitive attributes' do - expect_next_instance_of(ActiveSupport::ParameterFilter) do |instance| - expect(instance).to receive(:filter).exactly(4).times.and_call_original - end - - expect(result_hash).to include( - contexts: { - jwt: '[FILTERED]', - controller: 'GraphController#execute' - }, - tags: { - variables: '[FILTERED]', - deep_hash: { - sharedSecret: '[FILTERED]' - } - }, - user: { - email: 'a@a.com', - password: '[FILTERED]' - }, - extra: { - issue_url: 'http://gitlab.com/gitlab-org/gitlab-foss/-/issues/1', - my_token: '[FILTERED]', - another_token: '[FILTERED]' - } - ) - end - end - - context 'when request headers contains sensitive information' do - let(:exception) { RuntimeError.new } - let(:data) { {} } - - before do - event.rack_env = { - 'HTTP_AUTHORIZATION' => 'Bearer 123456', - 'HTTP_PRIVATE_TOKEN' => 'abcdef', - 'HTTP_GITLAB_WORKHORSE_PROXY_START' => 123456 - } - end - - it 'filters sensitive headers' do - expect(result_hash[:request][:headers]).to include( - 'Authorization' => '[FILTERED]', - 'Private-Token' => '[FILTERED]', - 'Gitlab-Workhorse-Proxy-Start' => '123456' - ) - end - end - end -end diff --git a/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb index bf8d31822a6..af5f11c9362 100644 --- a/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb @@ -95,18 +95,16 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor do end describe '.call' do - let(:exception) { StandardError.new('Test exception') } - let(:event) { Sentry.get_current_client.event_from_exception(exception) } - let(:result_hash) { described_class.call(event).to_hash } - - before do - Sentry.get_current_scope.update_from_options(**wrapped_value) - Sentry.get_current_scope.apply_to_event(event) + let(:required_options) do + { + configuration: Raven.configuration, + context: Raven.context, + breadcrumbs: Raven.breadcrumbs + } end - after do - Sentry.get_current_scope.clear - end + let(:event) { Raven::Event.new(required_options.merge(wrapped_value)) } + let(:result_hash) { described_class.call(event).to_hash } context 'when there is Sidekiq data' do let(:wrapped_value) { { extra: { sidekiq: value } } } diff --git a/spec/lib/gitlab/error_tracking_spec.rb b/spec/lib/gitlab/error_tracking_spec.rb index 50c85f76ce8..7ad1f52780a 100644 --- a/spec/lib/gitlab/error_tracking_spec.rb +++ b/spec/lib/gitlab/error_tracking_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -require 'sentry/transport/dummy_transport' +require 'raven/transports/dummy' RSpec.describe Gitlab::ErrorTracking do let(:exception) { RuntimeError.new('boom') } @@ -43,7 +43,7 @@ RSpec.describe Gitlab::ErrorTracking do } end - let(:sentry_event) { Sentry.get_current_client.transport.events.last } + let(:sentry_event) { Gitlab::Json.parse(Raven.client.transport.events.last[1]) } before do stub_sentry_settings @@ -53,7 +53,7 @@ RSpec.describe Gitlab::ErrorTracking do allow(I18n).to receive(:locale).and_return('en') described_class.configure do |config| - config.transport.transport_class = Sentry::DummyTransport + config.encoding = 'json' end end @@ -63,10 +63,6 @@ RSpec.describe Gitlab::ErrorTracking do end end - after do - ::Sentry.get_current_scope.clear - end - describe '.track_and_raise_for_dev_exception' do context 'when exceptions for dev should be raised' do before do @@ -74,7 +70,7 @@ RSpec.describe Gitlab::ErrorTracking do end it 'raises the exception' do - expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) + expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect do described_class.track_and_raise_for_dev_exception( @@ -92,7 +88,7 @@ RSpec.describe Gitlab::ErrorTracking do end it 'logs the exception with all attributes passed' do - expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) + expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) described_class.track_and_raise_for_dev_exception( exception, @@ -115,7 +111,7 @@ RSpec.describe Gitlab::ErrorTracking do describe '.track_and_raise_exception' do it 'always raises the exception' do - expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) + expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect do described_class.track_and_raise_for_dev_exception( @@ -143,14 +139,14 @@ RSpec.describe Gitlab::ErrorTracking do subject(:track_exception) { described_class.track_exception(exception, extra) } before do - allow(Sentry).to receive(:capture_exception).and_call_original + allow(Raven).to receive(:capture_exception).and_call_original allow(Gitlab::ErrorTracking::Logger).to receive(:error) end - it 'calls Sentry.capture_exception' do + it 'calls Raven.capture_exception' do track_exception - expect(Sentry).to have_received(:capture_exception).with( + expect(Raven).to have_received(:capture_exception).with( exception, sentry_payload ) @@ -176,31 +172,25 @@ RSpec.describe Gitlab::ErrorTracking do context 'the exception implements :sentry_extra_data' do let(:extra_info) { { event: 'explosion', size: :massive } } - - before do - allow(exception).to receive(:sentry_extra_data).and_return(extra_info) - end + let(:exception) { double(message: 'bang!', sentry_extra_data: extra_info, backtrace: caller, cause: nil) } it 'includes the extra data from the exception in the tracking information' do track_exception - expect(Sentry).to have_received(:capture_exception).with( + expect(Raven).to have_received(:capture_exception).with( exception, a_hash_including(extra: a_hash_including(extra_info)) ) end end context 'the exception implements :sentry_extra_data, which returns nil' do + let(:exception) { double(message: 'bang!', sentry_extra_data: nil, backtrace: caller, cause: nil) } let(:extra) { { issue_url: issue_url } } - before do - allow(exception).to receive(:sentry_extra_data).and_return(nil) - end - it 'just includes the other extra info' do track_exception - expect(Sentry).to have_received(:capture_exception).with( + expect(Raven).to have_received(:capture_exception).with( exception, a_hash_including(extra: a_hash_including(extra)) ) end @@ -212,7 +202,7 @@ RSpec.describe Gitlab::ErrorTracking do it 'injects the normalized sql query into extra' do track_exception - expect(sentry_event.extra[:sql]).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1') + expect(sentry_event.dig('extra', 'sql')).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1') end end @@ -222,7 +212,7 @@ RSpec.describe Gitlab::ErrorTracking do track_exception - expect(sentry_event.extra[:sql]).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1') + expect(sentry_event.dig('extra', 'sql')).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1') end end end @@ -231,27 +221,27 @@ RSpec.describe Gitlab::ErrorTracking do subject(:track_exception) { described_class.track_exception(exception, extra) } before do - allow(Sentry).to receive(:capture_exception).and_call_original + allow(Raven).to receive(:capture_exception).and_call_original allow(Gitlab::ErrorTracking::Logger).to receive(:error) end - context 'custom GitLab context when using Sentry.capture_exception directly' do - subject(:track_exception) { Sentry.capture_exception(exception) } + context 'custom GitLab context when using Raven.capture_exception directly' do + subject(:raven_capture_exception) { Raven.capture_exception(exception) } it 'merges a default set of tags into the existing tags' do - Sentry.set_tags(foo: 'bar') + allow(Raven.context).to receive(:tags).and_return(foo: 'bar') - track_exception + raven_capture_exception - expect(sentry_event.tags).to include(:correlation_id, :feature_category, :foo, :locale, :program) + expect(sentry_event['tags']).to include('correlation_id', 'feature_category', 'foo', 'locale', 'program') end it 'merges the current user information into the existing user information' do - Sentry.set_user(id: -1) + Raven.user_context(id: -1) - track_exception + raven_capture_exception - expect(sentry_event.user).to eq(id: -1, username: user.username) + expect(sentry_event['user']).to eq('id' => -1, 'username' => user.username) end end @@ -275,7 +265,7 @@ RSpec.describe Gitlab::ErrorTracking do it 'does not filter parameters when sending to Sentry' do track_exception - expect(sentry_event.extra[:sidekiq]['args']).to eq([1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value']) + expect(sentry_event.dig('extra', 'sidekiq', 'args')).to eq([1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value']) end end @@ -285,7 +275,7 @@ RSpec.describe Gitlab::ErrorTracking do it 'filters sensitive arguments before sending and logging' do track_exception - expect(sentry_event.extra[:sidekiq]['args']).to eq(['[FILTERED]', 1, 2]) + expect(sentry_event.dig('extra', 'sidekiq', 'args')).to eq(['[FILTERED]', 1, 2]) expect(Gitlab::ErrorTracking::Logger).to have_received(:error).with( hash_including( 'extra.sidekiq' => { @@ -305,8 +295,8 @@ RSpec.describe Gitlab::ErrorTracking do it 'sets the GRPC debug error string in the Sentry event and adds a custom fingerprint' do track_exception - expect(sentry_event.extra[:grpc_debug_error_string]).to eq('{"hello":1}') - expect(sentry_event.fingerprint).to eq(['GRPC::DeadlineExceeded', '4:unknown cause.']) + expect(sentry_event.dig('extra', 'grpc_debug_error_string')).to eq('{"hello":1}') + expect(sentry_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause.']) end end @@ -316,8 +306,8 @@ RSpec.describe Gitlab::ErrorTracking do it 'does not do any processing on the event' do track_exception - expect(sentry_event.extra).not_to include(:grpc_debug_error_string) - expect(sentry_event.fingerprint).to eq(['GRPC::DeadlineExceeded', '4:unknown cause']) + expect(sentry_event['extra']).not_to include('grpc_debug_error_string') + expect(sentry_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause']) end end end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb index c2479d20949..a10a8883591 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb @@ -18,14 +18,43 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi end describe '#schedule' do - it 'calls schedule on the strategy' do - expect do |block| - expect_next_instance_of(Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting) do |strategy| - expect(strategy).to receive(:schedule).with(job, &block) + shared_examples 'scheduling with deduplication class' do |strategy_class| + it 'calls schedule on the strategy' do + expect do |block| + expect_next_instance_of("Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::#{strategy_class}".constantize) do |strategy| + expect(strategy).to receive(:schedule).with(job, &block) + end + + duplicate_job.schedule(&block) + end.to yield_control + end + end + + it_behaves_like 'scheduling with deduplication class', 'UntilExecuting' + + context 'when the deduplication depends on a FF' do + before do + skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check + + allow(AuthorizedProjectsWorker).to receive(:get_deduplication_options).and_return(feature_flag: :my_feature_flag) + end + + context 'when the feature flag is enabled' do + before do + stub_feature_flags(my_feature_flag: true) end - duplicate_job.schedule(&block) - end.to yield_control + it_behaves_like 'scheduling with deduplication class', 'UntilExecuting' + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(my_feature_flag: false) + end + + it_behaves_like 'scheduling with deduplication class', 'None' + end end end diff --git a/spec/lib/peek/views/active_record_spec.rb b/spec/lib/peek/views/active_record_spec.rb index 9eeeca4de61..e5aae2822ed 100644 --- a/spec/lib/peek/views/active_record_spec.rb +++ b/spec/lib/peek/views/active_record_spec.rb @@ -5,16 +5,17 @@ require 'spec_helper' RSpec.describe Peek::Views::ActiveRecord, :request_store do subject { Peek.views.find { |v| v.instance_of?(Peek::Views::ActiveRecord) } } - let(:connection_1) { double(:connection) } - let(:connection_2) { double(:connection) } - let(:connection_3) { double(:connection) } + let(:connection_replica) { double(:connection_replica) } + let(:connection_primary_1) { double(:connection_primary) } + let(:connection_primary_2) { double(:connection_primary) } + let(:connection_unknown) { double(:connection_unknown) } let(:event_1) do { name: 'SQL', sql: 'SELECT * FROM users WHERE id = 10', cached: false, - connection: connection_1 + connection: connection_primary_1 } end @@ -23,7 +24,7 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do name: 'SQL', sql: 'SELECT * FROM users WHERE id = 10', cached: true, - connection: connection_2 + connection: connection_replica } end @@ -32,55 +33,141 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do name: 'SQL', sql: 'UPDATE users SET admin = true WHERE id = 10', cached: false, - connection: connection_3 + connection: connection_primary_2 + } + end + + let(:event_4) do + { + name: 'SCHEMA', + sql: 'SELECT VERSION()', + cached: false, + connection: connection_unknown } end before do allow(Gitlab::PerformanceBar).to receive(:enabled_for_request?).and_return(true) - allow(connection_1).to receive(:transaction_open?).and_return(false) - allow(connection_2).to receive(:transaction_open?).and_return(false) - allow(connection_3).to receive(:transaction_open?).and_return(true) + allow(connection_replica).to receive(:transaction_open?).and_return(false) + allow(connection_primary_1).to receive(:transaction_open?).and_return(false) + allow(connection_primary_2).to receive(:transaction_open?).and_return(true) + allow(connection_unknown).to receive(:transaction_open?).and_return(false) end - it 'subscribes and store data into peek views' do - Timecop.freeze(2021, 2, 23, 10, 0) do - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) + context 'when database load balancing is not enabled' do + it 'subscribes and store data into peek views' do + Timecop.freeze(2021, 2, 23, 10, 0) do + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4) + end + + expect(subject.results).to match( + calls: 4, + summary: { + "Cached" => 1, + "In a transaction" => 1 + }, + duration: '10000.00ms', + warnings: ["active-record duration: 10000.0 over 3000"], + details: contain_exactly( + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 1000.0, + sql: 'SELECT * FROM users WHERE id = 10' + ), + a_hash_including( + start: be_a(Time), + cached: 'Cached', + transaction: '', + duration: 2000.0, + sql: 'SELECT * FROM users WHERE id = 10' + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: 'In a transaction', + duration: 3000.0, + sql: 'UPDATE users SET admin = true WHERE id = 10' + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 4000.0, + sql: 'SELECT VERSION()' + ) + ) + ) + end + end + + context 'when database load balancing is enabled' do + before do + allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_replica).and_return(:replica) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_1).and_return(:primary) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_2).and_return(:primary) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_unknown).and_return(nil) end - expect(subject.results).to match( - calls: 3, - summary: { - "Cached" => 1, - "In a transaction" => 1 - }, - duration: '6000.00ms', - warnings: ["active-record duration: 6000.0 over 3000"], - details: contain_exactly( - a_hash_including( - start: be_a(Time), - cached: '', - transaction: '', - duration: 1000.0, - sql: 'SELECT * FROM users WHERE id = 10' - ), - a_hash_including( - start: be_a(Time), - cached: 'Cached', - transaction: '', - duration: 2000.0, - sql: 'SELECT * FROM users WHERE id = 10' - ), - a_hash_including( - start: be_a(Time), - cached: '', - transaction: 'In a transaction', - duration: 3000.0, - sql: 'UPDATE users SET admin = true WHERE id = 10' + it 'includes db role data' do + Timecop.freeze(2021, 2, 23, 10, 0) do + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4) + end + + expect(subject.results).to match( + calls: 4, + summary: { + "Cached" => 1, + "In a transaction" => 1, + "Primary" => 2, + "Replica" => 1, + "Unknown" => 1 + }, + duration: '10000.00ms', + warnings: ["active-record duration: 10000.0 over 3000"], + details: contain_exactly( + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 1000.0, + sql: 'SELECT * FROM users WHERE id = 10', + db_role: 'Primary' + ), + a_hash_including( + start: be_a(Time), + cached: 'Cached', + transaction: '', + duration: 2000.0, + sql: 'SELECT * FROM users WHERE id = 10', + db_role: 'Replica' + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: 'In a transaction', + duration: 3000.0, + sql: 'UPDATE users SET admin = true WHERE id = 10', + db_role: 'Primary' + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 4000.0, + sql: 'SELECT VERSION()', + db_role: 'Unknown' + ) ) ) - ) + end end end diff --git a/spec/lib/sidebars/projects/menus/learn_gitlab_menu_spec.rb b/spec/lib/sidebars/projects/menus/learn_gitlab_menu_spec.rb index ef5ae550551..231e5a850c2 100644 --- a/spec/lib/sidebars/projects/menus/learn_gitlab_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/learn_gitlab_menu_spec.rb @@ -27,7 +27,6 @@ RSpec.describe Sidebars::Projects::Menus::LearnGitlabMenu do { class: 'home', data: { - track_action: 'click_menu', track_property: tracking_category, track_label: 'learn_gitlab' } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index feb2f3630c1..69b4d752f4c 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -79,6 +79,32 @@ RSpec.describe CommitStatus do end end + describe '.updated_before' do + let!(:lookback) { 5.days.ago } + let!(:timeout) { 1.day.ago } + let!(:before_lookback) { lookback - 1.hour } + let!(:after_lookback) { lookback + 1.hour } + let!(:before_timeout) { timeout - 1.hour } + let!(:after_timeout) { timeout + 1.hour } + + subject { described_class.updated_before(lookback: lookback, timeout: timeout) } + + def create_build_with_set_timestamps(created_at:, updated_at:) + travel_to(created_at) { create(:ci_build, created_at: Time.current) }.tap do |build| + travel_to(updated_at) { build.update!(status: :failed) } + end + end + + it 'finds builds updated and created in the window between lookback and timeout' do + build_in_lookback_timeout_window = create_build_with_set_timestamps(created_at: after_lookback, updated_at: before_timeout) + build_outside_lookback_window = create_build_with_set_timestamps(created_at: before_lookback, updated_at: before_timeout) + build_outside_timeout_window = create_build_with_set_timestamps(created_at: after_lookback, updated_at: after_timeout) + + expect(subject).to contain_exactly(build_in_lookback_timeout_window) + expect(subject).not_to include(build_outside_lookback_window, build_outside_timeout_window) + end + end + describe '#processed' do subject { commit_status.processed } diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 3c690a767a6..ce0018d6d0d 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -require 'sentry/transport/dummy_transport' +require 'raven/transports/dummy' require_relative '../../../config/initializers/sentry' RSpec.describe API::Helpers do diff --git a/spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb index 9c95d1ff9d9..3760325675a 100644 --- a/spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb @@ -29,6 +29,34 @@ RSpec.shared_examples 'common trace features' do end end + describe '#read' do + context 'gitlab_ci_archived_trace_consistent_reads feature flag enabled' do + before do + stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: trace.job.project) + end + + it 'calls ::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking' do + expect(::Gitlab::Database::LoadBalancing::Sticking).to receive(:unstick_or_continue_sticking) + .with(described_class::LOAD_BALANCING_STICKING_NAMESPACE, trace.job.id) + .and_call_original + + trace.read { |stream| stream } + end + end + + context 'gitlab_ci_archived_trace_consistent_reads feature flag disabled' do + before do + stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: false) + end + + it 'does not call ::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking' do + expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:unstick_or_continue_sticking) + + trace.read { |stream| stream } + end + end + end + describe '#extract_coverage' do let(:regex) { '\(\d+.\d+\%\) covered' } @@ -253,6 +281,52 @@ RSpec.shared_examples 'common trace features' do describe '#archive!' do subject { trace.archive! } + context 'when live trace chunks exists' do + before do + # Build a trace_chunk manually + # It is possible to do so with trace.set but only if ci_enable_live_trace FF is enabled + # + # We need the job to have a trace_chunk because we only use #stick in + # the case where trace_chunks exist. + stream = Gitlab::Ci::Trace::Stream.new do + Gitlab::Ci::Trace::ChunkedIO.new(trace.job) + end + + stream.set(+"12\n34") + end + + # We check the before setup actually sets up job trace_chunks + it 'has job trace_chunks' do + expect(trace.job.trace_chunks).to be_present + end + + context 'gitlab_ci_archived_trace_consistent_reads feature flag enabled' do + before do + stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: trace.job.project) + end + + it 'calls ::Gitlab::Database::LoadBalancing::Sticking.stick' do + expect(::Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) + .with(described_class::LOAD_BALANCING_STICKING_NAMESPACE, trace.job.id) + .and_call_original + + subject + end + end + + context 'gitlab_ci_archived_trace_consistent_reads feature flag disabled' do + before do + stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: false) + end + + it 'does not call ::Gitlab::Database::LoadBalancing::Sticking.stick' do + expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:stick) + + subject + end + end + end + context 'when build status is success' do let!(:build) { create(:ci_build, :success, :trace_live) } diff --git a/spec/workers/concerns/worker_attributes_spec.rb b/spec/workers/concerns/worker_attributes_spec.rb index a654ecbd3e2..d4b17c65f46 100644 --- a/spec/workers/concerns/worker_attributes_spec.rb +++ b/spec/workers/concerns/worker_attributes_spec.rb @@ -62,6 +62,12 @@ RSpec.describe WorkerAttributes do end describe '.idempotent!' do + it 'sets `idempotent` attribute of the worker class to true' do + worker.idempotent! + + expect(worker.send(:class_attributes)[:idempotent]).to eq(true) + end + context 'when data consistency is not :always' do it 'raise exception' do worker.data_consistency(:sticky) @@ -71,4 +77,66 @@ RSpec.describe WorkerAttributes do end end end + + describe '.idempotent?' do + subject(:idempotent?) { worker.idempotent? } + + context 'when the worker is idempotent' do + before do + worker.idempotent! + end + + it { is_expected.to be_truthy } + end + + context 'when the worker is not idempotent' do + it { is_expected.to be_falsey } + end + end + + describe '.deduplicate' do + it 'sets deduplication_strategy and deduplication_options' do + worker.deduplicate(:until_executing, including_scheduled: true) + + expect(worker.send(:class_attributes)[:deduplication_strategy]).to eq(:until_executing) + expect(worker.send(:class_attributes)[:deduplication_options]).to eq(including_scheduled: true) + end + end + + describe '#deduplication_enabled?' do + subject(:deduplication_enabled?) { worker.deduplication_enabled? } + + context 'when no feature flag is set' do + before do + worker.deduplicate(:until_executing) + end + + it { is_expected.to eq(true) } + end + + context 'when feature flag is set' do + before do + skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check + + worker.deduplicate(:until_executing, feature_flag: :my_feature_flag) + end + + context 'when the FF is enabled' do + before do + stub_feature_flags(my_feature_flag: true) + end + + it { is_expected.to eq(true) } + end + + context 'when the FF is disabled' do + before do + stub_feature_flags(my_feature_flag: false) + end + + it { is_expected.to eq(false) } + end + end + end end diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index 24d3b6fadf5..84b2d87494e 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -9,12 +9,17 @@ RSpec.describe StuckCiJobsWorker do let!(:job) { create :ci_build, runner: runner } let(:worker_lease_key) { StuckCiJobsWorker::EXCLUSIVE_LEASE_KEY } let(:worker_lease_uuid) { SecureRandom.uuid } + let(:created_at) { } + let(:updated_at) { } subject(:worker) { described_class.new } before do stub_exclusive_lease(worker_lease_key, worker_lease_uuid) - job.update!(status: status, updated_at: updated_at) + job_attributes = { status: status } + job_attributes[:created_at] = created_at if created_at + job_attributes[:updated_at] = updated_at if updated_at + job.update!(job_attributes) end shared_examples 'job is dropped' do @@ -63,22 +68,70 @@ RSpec.describe StuckCiJobsWorker do allow_any_instance_of(Ci::Build).to receive(:stuck?).and_return(false) end - context 'when job was not updated for more than 1 day ago' do - let(:updated_at) { 2.days.ago } + context 'when job was updated_at more than 1 day ago' do + let(:updated_at) { 1.5.days.ago } - it_behaves_like 'job is dropped' + context 'when created_at is the same as updated_at' do + let(:created_at) { 1.5.days.ago } + + it_behaves_like 'job is dropped' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } + + it_behaves_like 'job is dropped' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end - context 'when job was updated in less than 1 day ago' do + context 'when job was updated less than 1 day ago' do let(:updated_at) { 6.hours.ago } - it_behaves_like 'job is unchanged' + context 'when created_at is the same as updated_at' do + let(:created_at) { 1.5.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end - context 'when job was not updated for more than 1 hour ago' do + context 'when job was updated more than 1 hour ago' do let(:updated_at) { 2.hours.ago } - it_behaves_like 'job is unchanged' + context 'when created_at is the same as updated_at' do + let(:created_at) { 2.hours.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end end @@ -87,17 +140,48 @@ RSpec.describe StuckCiJobsWorker do allow_any_instance_of(Ci::Build).to receive(:stuck?).and_return(true) end - context 'when job was not updated for more than 1 hour ago' do - let(:updated_at) { 2.hours.ago } + context 'when job was updated_at more than 1 hour ago' do + let(:updated_at) { 1.5.hours.ago } + + context 'when created_at is the same as updated_at' do + let(:created_at) { 1.5.hours.ago } + + it_behaves_like 'job is dropped' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } - it_behaves_like 'job is dropped' + it_behaves_like 'job is dropped' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end - context 'when job was updated in less than 1 - hour ago' do + context 'when job was updated in less than 1 hour ago' do let(:updated_at) { 30.minutes.ago } - it_behaves_like 'job is unchanged' + context 'when created_at is the same as updated_at' do + let(:created_at) { 30.minutes.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 2.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end end end @@ -105,7 +189,7 @@ RSpec.describe StuckCiJobsWorker do context 'when job is running' do let(:status) { 'running' } - context 'when job was not updated for more than 1 hour ago' do + context 'when job was updated_at more than an hour ago' do let(:updated_at) { 2.hours.ago } it_behaves_like 'job is dropped' @@ -123,7 +207,23 @@ RSpec.describe StuckCiJobsWorker do let(:status) { status } let(:updated_at) { 2.days.ago } - it_behaves_like 'job is unchanged' + context 'when created_at is the same as updated_at' do + let(:created_at) { 2.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end end |