diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-30 15:14:17 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-30 15:14:17 +0000 |
| commit | 3fe9588b1c1c4fb58f8ba8e9c27244fc2fc1c103 (patch) | |
| tree | d19448d010ff9d58fed14846736ee358fb6b3327 | |
| parent | ad8eea383406037a207c80421e6e4bfa357f8044 (diff) | |
| download | gitlab-ce-3fe9588b1c1c4fb58f8ba8e9c27244fc2fc1c103.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
112 files changed, 2337 insertions, 156 deletions
diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index ddfdf72cf99..61653d798a9 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -6,8 +6,8 @@ /doc/ @axil @marcia @eread @mikelewis # Frontend maintainers should see everything in `app/assets/` -app/assets/ @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina -*.scss @annabeldunstone @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina +app/assets/ @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina @iamphill +*.scss @annabeldunstone @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina @iamphill # Database maintainers should review changes in `db/` db/ @gitlab-org/maintainers/database diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 2ab492c16b2..a71c4ea2f1d 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -122,6 +122,7 @@ schedule:review-build-cng: - source scripts/utils.sh - install_api_client_dependencies_with_apk - source scripts/review_apps/review-apps.sh + - export REVIEW_APP_CONFIG_CHANGED=$(base_config_changed) script: - date - check_kube_domain diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md index b939be178be..cd501f8583a 100644 --- a/CHANGELOG-EE.md +++ b/CHANGELOG-EE.md @@ -1,5 +1,17 @@ Please view this file on the master branch, on stable branches it's out of date. +## 12.4.1 + +### Security (6 changes) + +- Do not display project labels that are not visible for user accessing group labels. +- Do not index system notes for issue update. +- Redact search results based on Ability.allowed?. +- Do not show private cross references in epic notes. +- Filter out packages the user does'nt have permission to see at group level. +- Fixes a Open Redirect issue in `InternalRedirect`. + + ## 12.4.0 ### Security (2 changes) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16a36724b4f..64f7957860c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,12 @@ entry. ## 12.4.1 -### Security (12 changes) +### Security (14 changes) - Standardize error response when route is missing. - Do not display project labels that are not visible for user accessing group labels. - Show cross-referenced label and milestones in issues' activities only to authorized users. +- Show cross-referenced label and milestones in issues' activities only to authorized users. - Analyze incoming GraphQL queries and check for recursion. - Disallow unprivileged users from commenting on private repository commits. - Don't allow maintainers of a target project to delete the source branch of a merge request from a fork. @@ -17,6 +18,7 @@ entry. - Return 404 on LFS request if project doesn't exist. - Mask sentry auth token in Error Tracking dashboard. - Fixes a Open Redirect issue in `InternalRedirect`. +- Remove deploy access level when project/group link is deleted. - Sanitize all wiki markup formats with GitLab sanitization pipelines. diff --git a/app/assets/javascripts/monitoring/components/charts/anomaly.vue b/app/assets/javascripts/monitoring/components/charts/anomaly.vue new file mode 100644 index 00000000000..8eeac737a11 --- /dev/null +++ b/app/assets/javascripts/monitoring/components/charts/anomaly.vue @@ -0,0 +1,227 @@ +<script> +import { flatten, isNumber } from 'underscore'; +import { GlLineChart, GlChartSeriesLabel } from '@gitlab/ui/dist/charts'; +import { roundOffFloat } from '~/lib/utils/common_utils'; +import { hexToRgb } from '~/lib/utils/color_utils'; +import { areaOpacityValues, symbolSizes, colorValues } from '../../constants'; +import { graphDataValidatorForAnomalyValues } from '../../utils'; +import MonitorTimeSeriesChart from './time_series.vue'; + +/** + * Series indexes + */ +const METRIC = 0; +const UPPER = 1; +const LOWER = 2; + +/** + * Boundary area appearance + */ +const AREA_COLOR = colorValues.anomalyAreaColor; +const AREA_OPACITY = areaOpacityValues.default; +const AREA_COLOR_RGBA = `rgba(${hexToRgb(AREA_COLOR).join(',')},${AREA_OPACITY})`; + +/** + * The anomaly component highlights when a metric shows + * some anomalous behavior. + * + * It shows both a metric line and a boundary band in a + * time series chart, the boundary band shows the normal + * range of values the metric should take. + * + * This component accepts 3 queries, which contain the + * "metric", "upper" limit and "lower" limit. + * + * The upper and lower series are "stacked areas" visually + * to create the boundary band, and if any "metric" value + * is outside this band, it is highlighted to warn users. + * + * The boundary band stack must be painted above the 0 line + * so the area is shown correctly. If any of the values of + * the data are negative, the chart data is shifted to be + * above 0 line. + * + * The data passed to the time series is will always be + * positive, but reformatted to show the original values of + * data. + * + */ +export default { + components: { + GlLineChart, + GlChartSeriesLabel, + MonitorTimeSeriesChart, + }, + inheritAttrs: false, + props: { + graphData: { + type: Object, + required: true, + validator: graphDataValidatorForAnomalyValues, + }, + }, + computed: { + series() { + return this.graphData.queries.map(query => { + const values = query.result[0] ? query.result[0].values : []; + return { + label: query.label, + data: values.filter(([, value]) => !Number.isNaN(value)), + }; + }); + }, + /** + * If any of the values of the data is negative, the + * chart data is shifted to the lowest value + * + * This offset is the lowest value. + */ + yOffset() { + const values = flatten(this.series.map(ser => ser.data.map(([, y]) => y))); + const min = values.length ? Math.floor(Math.min(...values)) : 0; + return min < 0 ? -min : 0; + }, + metricData() { + const originalMetricQuery = this.graphData.queries[0]; + + const metricQuery = { ...originalMetricQuery }; + metricQuery.result[0].values = metricQuery.result[0].values.map(([x, y]) => [ + x, + y + this.yOffset, + ]); + return { + ...this.graphData, + type: 'line-chart', + queries: [metricQuery], + }; + }, + metricSeriesConfig() { + return { + type: 'line', + symbol: 'circle', + symbolSize: (val, params) => { + if (this.isDatapointAnomaly(params.dataIndex)) { + return symbolSizes.anomaly; + } + // 0 causes echarts to throw an error, use small number instead + // see https://gitlab.com/gitlab-org/gitlab-ui/issues/423 + return 0.001; + }, + showSymbol: true, + itemStyle: { + color: params => { + if (this.isDatapointAnomaly(params.dataIndex)) { + return colorValues.anomalySymbol; + } + return colorValues.primaryColor; + }, + }, + }; + }, + chartOptions() { + const [, upperSeries, lowerSeries] = this.series; + const calcOffsetY = (data, offsetCallback) => + data.map((value, dataIndex) => { + const [x, y] = value; + return [x, y + offsetCallback(dataIndex)]; + }); + + const yAxisWithOffset = { + name: this.yAxisLabel, + axisLabel: { + formatter: num => roundOffFloat(num - this.yOffset, 3).toString(), + }, + }; + + /** + * Boundary is rendered by 2 series: An invisible + * series (opacity: 0) stacked on a visible one. + * + * Order is important, lower boundary is stacked + * *below* the upper boundary. + */ + const boundarySeries = []; + + if (upperSeries.data.length && lowerSeries.data.length) { + // Lower boundary, plus the offset if negative values + boundarySeries.push( + this.makeBoundarySeries({ + name: this.formatLegendLabel(lowerSeries), + data: calcOffsetY(lowerSeries.data, () => this.yOffset), + }), + ); + // Upper boundary, minus the lower boundary + boundarySeries.push( + this.makeBoundarySeries({ + name: this.formatLegendLabel(upperSeries), + data: calcOffsetY(upperSeries.data, i => -this.yValue(LOWER, i)), + areaStyle: { + color: AREA_COLOR, + opacity: AREA_OPACITY, + }, + }), + ); + } + return { yAxis: yAxisWithOffset, series: boundarySeries }; + }, + }, + methods: { + formatLegendLabel(query) { + return query.label; + }, + yValue(seriesIndex, dataIndex) { + const d = this.series[seriesIndex].data[dataIndex]; + return d && d[1]; + }, + yValueFormatted(seriesIndex, dataIndex) { + const y = this.yValue(seriesIndex, dataIndex); + return isNumber(y) ? y.toFixed(3) : ''; + }, + isDatapointAnomaly(dataIndex) { + const yVal = this.yValue(METRIC, dataIndex); + const yUpper = this.yValue(UPPER, dataIndex); + const yLower = this.yValue(LOWER, dataIndex); + return (isNumber(yUpper) && yVal > yUpper) || (isNumber(yLower) && yVal < yLower); + }, + makeBoundarySeries(series) { + const stackKey = 'anomaly-boundary-series-stack'; + return { + type: 'line', + stack: stackKey, + lineStyle: { + width: 0, + color: AREA_COLOR_RGBA, // legend color + }, + color: AREA_COLOR_RGBA, // tooltip color + symbol: 'none', + ...series, + }; + }, + }, +}; +</script> + +<template> + <monitor-time-series-chart + v-bind="$attrs" + :graph-data="metricData" + :option="chartOptions" + :series-config="metricSeriesConfig" + > + <slot></slot> + <template v-slot:tooltipContent="slotProps"> + <div + v-for="(content, seriesIndex) in slotProps.tooltip.content" + :key="seriesIndex" + class="d-flex justify-content-between" + > + <gl-chart-series-label :color="content.color"> + {{ content.name }} + </gl-chart-series-label> + <div class="prepend-left-32"> + {{ yValueFormatted(seriesIndex, content.dataIndex) }} + </div> + </div> + </template> + </monitor-time-series-chart> +</template> diff --git a/app/assets/javascripts/monitoring/components/charts/time_series.vue b/app/assets/javascripts/monitoring/components/charts/time_series.vue index 434debb67f5..6a88c8a5ee3 100644 --- a/app/assets/javascripts/monitoring/components/charts/time_series.vue +++ b/app/assets/javascripts/monitoring/components/charts/time_series.vue @@ -1,12 +1,20 @@ <script> import { s__, __ } from '~/locale'; +import _ from 'underscore'; import { GlLink, GlButton, GlTooltip, GlResizeObserverDirective } from '@gitlab/ui'; import { GlAreaChart, GlLineChart, GlChartSeriesLabel } from '@gitlab/ui/dist/charts'; import dateFormat from 'dateformat'; import { roundOffFloat } from '~/lib/utils/common_utils'; import { getSvgIconPathContent } from '~/lib/utils/icon_utils'; import Icon from '~/vue_shared/components/icon.vue'; -import { chartHeight, graphTypes, lineTypes, symbolSizes, dateFormats } from '../../constants'; +import { + chartHeight, + graphTypes, + lineTypes, + lineWidths, + symbolSizes, + dateFormats, +} from '../../constants'; import { makeDataSeries } from '~/helpers/monitor_helper'; import { graphDataValidatorForValues } from '../../utils'; @@ -30,6 +38,16 @@ export default { required: true, validator: graphDataValidatorForValues.bind(null, false), }, + option: { + type: Object, + required: false, + default: () => ({}), + }, + seriesConfig: { + type: Object, + required: false, + default: () => ({}), + }, deploymentData: { type: Array, required: false, @@ -96,29 +114,35 @@ export default { const lineWidth = appearance && appearance.line && appearance.line.width ? appearance.line.width - : undefined; + : lineWidths.default; const areaStyle = { opacity: appearance && appearance.area && typeof appearance.area.opacity === 'number' ? appearance.area.opacity : undefined, }; - const series = makeDataSeries(query.result, { name: this.formatLegendLabel(query), lineStyle: { type: lineType, width: lineWidth, + color: this.primaryColor, }, showSymbol: false, areaStyle: this.graphData.type === 'area-chart' ? areaStyle : undefined, + ...this.seriesConfig, }); return acc.concat(series); }, []); }, + chartOptionSeries() { + return (this.option.series || []).concat(this.scatterSeries ? [this.scatterSeries] : []); + }, chartOptions() { + const option = _.omit(this.option, 'series'); return { + series: this.chartOptionSeries, xAxis: { name: __('Time'), type: 'time', @@ -135,8 +159,8 @@ export default { formatter: num => roundOffFloat(num, 3).toString(), }, }, - series: this.scatterSeries, dataZoom: [this.dataZoomConfig], + ...option, }; }, dataZoomConfig() { @@ -144,6 +168,14 @@ export default { return handleIcon ? { handleIcon } : {}; }, + /** + * This method returns the earliest time value in all series of a chart. + * Takes a chart data with data to populate a timeseries. + * data should be an array of data points [t, y] where t is a ISO formatted date, + * and is sorted by t (time). + * @returns {(String|null)} earliest x value from all series, or null when the + * chart series data is empty. + */ earliestDatapoint() { return this.chartData.reduce((acc, series) => { const { data } = series; @@ -230,10 +262,11 @@ export default { this.tooltip.sha = deploy.sha.substring(0, 8); this.tooltip.commitUrl = deploy.commitUrl; } else { - const { seriesName, color } = dataPoint; + const { seriesName, color, dataIndex } = dataPoint; const value = yVal.toFixed(3); this.tooltip.content.push({ name: seriesName, + dataIndex, value, color, }); @@ -306,23 +339,27 @@ export default { </template> <template v-else> <template slot="tooltipTitle"> - <div class="text-nowrap"> - {{ tooltip.title }} - </div> + <slot name="tooltipTitle"> + <div class="text-nowrap"> + {{ tooltip.title }} + </div> + </slot> </template> <template slot="tooltipContent"> - <div - v-for="(content, key) in tooltip.content" - :key="key" - class="d-flex justify-content-between" - > - <gl-chart-series-label :color="isMultiSeries ? content.color : ''"> - {{ content.name }} - </gl-chart-series-label> - <div class="prepend-left-32"> - {{ content.value }} + <slot name="tooltipContent" :tooltip="tooltip"> + <div + v-for="(content, key) in tooltip.content" + :key="key" + class="d-flex justify-content-between" + > + <gl-chart-series-label :color="isMultiSeries ? content.color : ''"> + {{ content.name }} + </gl-chart-series-label> + <div class="prepend-left-32"> + {{ content.value }} + </div> </div> - </div> + </slot> </template> </template> </component> diff --git a/app/assets/javascripts/monitoring/components/panel_type.vue b/app/assets/javascripts/monitoring/components/panel_type.vue index 2c56966f120..e3f99dbda9a 100644 --- a/app/assets/javascripts/monitoring/components/panel_type.vue +++ b/app/assets/javascripts/monitoring/components/panel_type.vue @@ -11,6 +11,7 @@ import { } from '@gitlab/ui'; import Icon from '~/vue_shared/components/icon.vue'; import MonitorTimeSeriesChart from './charts/time_series.vue'; +import MonitorAnomalyChart from './charts/anomaly.vue'; import MonitorSingleStatChart from './charts/single_stat.vue'; import MonitorEmptyChart from './charts/empty_chart.vue'; import TrackEventDirective from '~/vue_shared/directives/track_event'; @@ -19,7 +20,6 @@ import { downloadCSVOptions, generateLinkToChartOptions } from '../utils'; export default { components: { MonitorSingleStatChart, - MonitorTimeSeriesChart, MonitorEmptyChart, Icon, GlDropdown, @@ -67,6 +67,12 @@ export default { const data = new Blob([this.csvText], { type: 'text/plain' }); return window.URL.createObjectURL(data); }, + monitorChartComponent() { + if (this.isPanelType('anomaly-chart')) { + return MonitorAnomalyChart; + } + return MonitorTimeSeriesChart; + }, }, methods: { getGraphAlerts(queries) { @@ -93,13 +99,14 @@ export default { v-if="isPanelType('single-stat') && graphDataHasMetrics" :graph-data="graphData" /> - <monitor-time-series-chart + <component + :is="monitorChartComponent" v-else-if="graphDataHasMetrics" :graph-data="graphData" :deployment-data="deploymentData" :project-path="projectPath" :thresholds="getGraphAlertValues(graphData.queries)" - group-id="monitor-area-chart" + group-id="panel-type-chart" > <div class="d-flex align-items-center"> <alert-widget @@ -141,6 +148,6 @@ export default { </gl-dropdown-item> </gl-dropdown> </div> - </monitor-time-series-chart> + </component> <monitor-empty-chart v-else :graph-title="graphData.title" /> </template> diff --git a/app/assets/javascripts/monitoring/constants.js b/app/assets/javascripts/monitoring/constants.js index 2836fe4fc26..1a1fcdd0e66 100644 --- a/app/assets/javascripts/monitoring/constants.js +++ b/app/assets/javascripts/monitoring/constants.js @@ -14,13 +14,28 @@ export const graphTypes = { }; export const symbolSizes = { + anomaly: 8, default: 14, }; +export const areaOpacityValues = { + default: 0.2, +}; + +export const colorValues = { + primaryColor: '#1f78d1', // $blue-500 (see variables.scss) + anomalySymbol: '#db3b21', + anomalyAreaColor: '#1f78d1', +}; + export const lineTypes = { default: 'solid', }; +export const lineWidths = { + default: 2, +}; + export const timeWindows = { thirtyMinutes: __('30 minutes'), threeHours: __('3 hours'), diff --git a/app/assets/javascripts/monitoring/utils.js b/app/assets/javascripts/monitoring/utils.js index 00f188c1d5a..6747306a6d9 100644 --- a/app/assets/javascripts/monitoring/utils.js +++ b/app/assets/javascripts/monitoring/utils.js @@ -131,4 +131,20 @@ export const downloadCSVOptions = title => { return { category, action, label: 'Chart title', property: title }; }; +/** + * This function validates the graph data contains exactly 3 queries plus + * value validations from graphDataValidatorForValues. + * @param {Object} isValues + * @param {Object} graphData the graph data response from a prometheus request + * @returns {boolean} true if the data is valid + */ +export const graphDataValidatorForAnomalyValues = graphData => { + const anomalySeriesCount = 3; // metric, upper, lower + return ( + graphData.queries && + graphData.queries.length === anomalySeriesCount && + graphDataValidatorForValues(false, graphData) + ); +}; + export default {}; diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index fcc57da0649..4802cc2ad25 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -1,7 +1,6 @@ <script> -/* eslint-disable @gitlab/vue-i18n/no-bare-strings */ import settingsMixin from 'ee_else_ce/pages/projects/shared/permissions/mixins/settings_pannel_mixin'; -import { __ } from '~/locale'; +import { s__ } from '~/locale'; import projectFeatureSetting from './project_feature_setting.vue'; import projectFeatureToggle from '~/vue_shared/components/toggle_button.vue'; import projectSettingRow from './project_setting_row.vue'; @@ -13,7 +12,7 @@ import { } from '../constants'; import { toggleHiddenClassBySelector } from '../external'; -const PAGE_FEATURE_ACCESS_LEVEL = __('Everyone'); +const PAGE_FEATURE_ACCESS_LEVEL = s__('ProjectSettings|Everyone'); export default { components: { @@ -207,7 +206,10 @@ export default { <template> <div> <div class="project-visibility-setting"> - <project-setting-row :help-path="visibilityHelpPath" label="Project visibility"> + <project-setting-row + :help-path="visibilityHelpPath" + :label="s__('ProjectSettings|Project visibility')" + > <div class="project-feature-controls"> <div class="select-wrapper"> <select @@ -220,17 +222,17 @@ export default { <option :value="visibilityOptions.PRIVATE" :disabled="!visibilityAllowed(visibilityOptions.PRIVATE)" - >{{ __('Private') }}</option + >{{ s__('ProjectSettings|Private') }}</option > <option :value="visibilityOptions.INTERNAL" :disabled="!visibilityAllowed(visibilityOptions.INTERNAL)" - >{{ __('Internal') }}</option + >{{ s__('ProjectSettings|Internal') }}</option > <option :value="visibilityOptions.PUBLIC" :disabled="!visibilityAllowed(visibilityOptions.PUBLIC)" - >{{ __('Public') }}</option + >{{ s__('ProjectSettings|Public') }}</option > </select> <i aria-hidden="true" data-hidden="true" class="fa fa-chevron-down"></i> @@ -243,14 +245,15 @@ export default { type="hidden" name="project[request_access_enabled]" /> - <input v-model="requestAccessEnabled" type="checkbox" /> Allow users to request access + <input v-model="requestAccessEnabled" type="checkbox" /> + {{ s__('ProjectSettings|Allow users to request access') }} </label> </project-setting-row> </div> <div :class="{ 'highlight-changes': highlightChangesClass }" class="project-feature-settings"> <project-setting-row - label="Issues" - help-text="Lightweight issue tracking system for this project" + :label="s__('ProjectSettings|Issues')" + :help-text="s__('ProjectSettings|Lightweight issue tracking system for this project')" > <project-feature-setting v-model="issuesAccessLevel" @@ -258,7 +261,10 @@ export default { name="project[project_feature_attributes][issues_access_level]" /> </project-setting-row> - <project-setting-row label="Repository" help-text="View and edit files in this project"> + <project-setting-row + :label="s__('ProjectSettings|Repository')" + :help-text="s__('ProjectSettings|View and edit files in this project')" + > <project-feature-setting v-model="repositoryAccessLevel" :options="featureAccessLevelOptions" @@ -267,8 +273,8 @@ export default { </project-setting-row> <div class="project-feature-setting-group"> <project-setting-row - label="Merge requests" - help-text="Submit changes to be merged upstream" + :label="s__('ProjectSettings|Merge requests')" + :help-text="s__('ProjectSettings|Submit changes to be merged upstream')" > <project-feature-setting v-model="mergeRequestsAccessLevel" @@ -277,7 +283,10 @@ export default { name="project[project_feature_attributes][merge_requests_access_level]" /> </project-setting-row> - <project-setting-row label="Pipelines" help-text="Build, test, and deploy your changes"> + <project-setting-row + :label="s__('ProjectSettings|Pipelines')" + :help-text="s__('ProjectSettings|Build, test, and deploy your changes')" + > <project-feature-setting v-model="buildsAccessLevel" :options="repoFeatureAccessLevelOptions" @@ -288,11 +297,17 @@ export default { <project-setting-row v-if="registryAvailable" :help-path="registryHelpPath" - label="Container registry" - help-text="Every project can have its own space to store its Docker images" + :label="s__('ProjectSettings|Container registry')" + :help-text=" + s__('ProjectSettings|Every project can have its own space to store its Docker images') + " > <div v-if="showContainerRegistryPublicNote" class="text-muted"> - {{ __('Note: the container registry is always visible when a project is public') }} + {{ + s__( + 'ProjectSettings|Note: the container registry is always visible when a project is public', + ) + }} </div> <project-feature-toggle v-model="containerRegistryEnabled" @@ -303,8 +318,10 @@ export default { <project-setting-row v-if="lfsAvailable" :help-path="lfsHelpPath" - label="Git Large File Storage" - help-text="Manages large files such as audio, video, and graphics files" + :label="s__('ProjectSettings|Git Large File Storage')" + :help-text=" + s__('ProjectSettings|Manages large files such as audio, video, and graphics files') + " > <project-feature-toggle v-model="lfsEnabled" @@ -315,8 +332,10 @@ export default { <project-setting-row v-if="packagesAvailable" :help-path="packagesHelpPath" - label="Packages" - help-text="Every project can have its own space to store its packages" + :label="s__('ProjectSettings|Packages')" + :help-text=" + s__('ProjectSettings|Every project can have its own space to store its packages') + " > <project-feature-toggle v-model="packagesEnabled" @@ -325,7 +344,10 @@ export default { /> </project-setting-row> </div> - <project-setting-row label="Wiki" help-text="Pages for project documentation"> + <project-setting-row + :label="s__('ProjectSettings|Wiki')" + :help-text="s__('ProjectSettings|Pages for project documentation')" + > <project-feature-setting v-model="wikiAccessLevel" :options="featureAccessLevelOptions" @@ -333,8 +355,8 @@ export default { /> </project-setting-row> <project-setting-row - label="Snippets" - help-text="Share code pastes with others out of Git repository" + :label="s__('ProjectSettings|Snippets')" + :help-text="s__('ProjectSettings|Share code pastes with others out of Git repository')" > <project-feature-setting v-model="snippetsAccessLevel" @@ -346,7 +368,9 @@ export default { v-if="pagesAvailable && pagesAccessControlEnabled" :help-path="pagesHelpPath" :label="s__('ProjectSettings|Pages')" - :help-text="__('With GitLab Pages you can host your static websites on GitLab')" + :help-text=" + s__('ProjectSettings|With GitLab Pages you can host your static websites on GitLab') + " > <project-feature-setting v-model="pagesAccessLevel" @@ -358,10 +382,13 @@ export default { <project-setting-row v-if="canDisableEmails" class="mb-3"> <label class="js-emails-disabled"> <input :value="emailsDisabled" type="hidden" name="project[emails_disabled]" /> - <input v-model="emailsDisabled" type="checkbox" /> {{ __('Disable email notifications') }} + <input v-model="emailsDisabled" type="checkbox" /> + {{ s__('ProjectSettings|Disable email notifications') }} </label> <span class="form-text text-muted">{{ - __('This setting will override user notification preferences for all project members.') + s__( + 'ProjectSettings|This setting will override user notification preferences for all project members.', + ) }}</span> </project-setting-row> </div> diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index 171841178a3..81ae5143082 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -97,11 +97,13 @@ export default { }, }, methods: { - openRow() { - if (this.isFolder) { + openRow(e) { + if (e.target.tagName === 'A') return; + + if (this.isFolder && !e.metaKey) { this.$router.push(this.routerLinkTo); } else { - visitUrl(this.url); + visitUrl(this.url, e.metaKey); } }, }, diff --git a/app/controllers/groups/group_links_controller.rb b/app/controllers/groups/group_links_controller.rb new file mode 100644 index 00000000000..7965311c5f1 --- /dev/null +++ b/app/controllers/groups/group_links_controller.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class Groups::GroupLinksController < Groups::ApplicationController + before_action :check_feature_flag! + before_action :authorize_admin_group! + + def create + shared_with_group = Group.find(params[:shared_with_group_id]) if params[:shared_with_group_id].present? + + if shared_with_group + result = Groups::GroupLinks::CreateService + .new(shared_with_group, current_user, group_link_create_params) + .execute(group) + + return render_404 if result[:http_status] == 404 + + flash[:alert] = result[:message] if result[:status] == :error + else + flash[:alert] = _('Please select a group.') + end + + redirect_to group_group_members_path(group) + end + + private + + def group_link_create_params + params.permit(:shared_group_access, :expires_at) + end + + def check_feature_flag! + render_404 unless Feature.enabled?(:share_group_with_group) + end +end diff --git a/app/models/concerns/worker_attributes.rb b/app/models/concerns/worker_attributes.rb index af40e9e3b19..506215ca9ed 100644 --- a/app/models/concerns/worker_attributes.rb +++ b/app/models/concerns/worker_attributes.rb @@ -3,6 +3,10 @@ module WorkerAttributes extend ActiveSupport::Concern + # Resource boundaries that workers can declare through the + # `worker_resource_boundary` attribute + VALID_RESOURCE_BOUNDARIES = [:memory, :cpu, :unknown].freeze + class_methods do def feature_category(value) raise "Invalid category. Use `feature_category_not_owned!` to mark a worker as not owned" if value == :not_owned @@ -24,6 +28,48 @@ module WorkerAttributes get_worker_attribute(:feature_category) == :not_owned end + # This should be set for jobs that need to be run immediately, or, if + # they are delayed, risk creating inconsistencies in the application + # that could being perceived by the user as incorrect behavior + # (ie, a bug) + # See doc/development/sidekiq_style_guide.md#Latency-Sensitive-Jobs + # for details + def latency_sensitive_worker! + worker_attributes[:latency_sensitive] = true + end + + # Returns a truthy value if the worker is latency sensitive. + # See doc/development/sidekiq_style_guide.md#Latency-Sensitive-Jobs + # for details + def latency_sensitive_worker? + worker_attributes[:latency_sensitive] + end + + # Set this attribute on a job when it will call to services outside of the + # application, such as 3rd party applications, other k8s clusters etc See + # doc/development/sidekiq_style_guide.md#Jobs-with-External-Dependencies for + # details + def worker_has_external_dependencies! + worker_attributes[:external_dependencies] = true + end + + # Returns a truthy value if the worker has external dependencies. + # See doc/development/sidekiq_style_guide.md#Jobs-with-External-Dependencies + # for details + def worker_has_external_dependencies? + worker_attributes[:external_dependencies] + end + + def worker_resource_boundary(boundary) + raise "Invalid boundary" unless VALID_RESOURCE_BOUNDARIES.include? boundary + + worker_attributes[:resource_boundary] = boundary + end + + def get_worker_resource_boundary + worker_attributes[:resource_boundary] || :unknown + end + protected # Returns a worker attribute declared on this class or its parent class. diff --git a/app/models/group.rb b/app/models/group.rb index 042201ffa14..27e4d709823 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -30,6 +30,10 @@ class Group < Namespace has_many :members_and_requesters, as: :source, class_name: 'GroupMember' has_many :milestones + has_many :shared_group_links, foreign_key: :shared_with_group_id, class_name: 'GroupGroupLink' + has_many :shared_with_group_links, foreign_key: :shared_group_id, class_name: 'GroupGroupLink' + has_many :shared_groups, through: :shared_group_links, source: :shared_group + has_many :shared_with_groups, through: :shared_with_group_links, source: :shared_with_group has_many :project_group_links, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :shared_projects, through: :project_group_links, source: :project @@ -376,11 +380,12 @@ class Group < Namespace return GroupMember::OWNER if user.admin? - members_with_parents - .where(user_id: user) - .reorder(access_level: :desc) - .first&. - access_level || GroupMember::NO_ACCESS + max_member_access = members_with_parents.where(user_id: user) + .reorder(access_level: :desc) + .first + &.access_level + + max_member_access || max_member_access_for_user_from_shared_groups(user) || GroupMember::NO_ACCESS end def mattermost_team_params @@ -474,6 +479,26 @@ class Group < Namespace errors.add(:visibility_level, "#{visibility} is not allowed since there are sub-groups with higher visibility.") end + def max_member_access_for_user_from_shared_groups(user) + return unless Feature.enabled?(:share_group_with_group) + + group_group_link_table = GroupGroupLink.arel_table + group_member_table = GroupMember.arel_table + + group_group_links_query = GroupGroupLink.where(shared_group_id: self_and_ancestors_ids) + cte = Gitlab::SQL::CTE.new(:group_group_links_cte, group_group_links_query) + + link = GroupGroupLink + .with(cte.to_arel) + .from([group_member_table, cte.alias_to(group_group_link_table)]) + .where(group_member_table[:user_id].eq(user.id)) + .where(group_member_table[:source_id].eq(group_group_link_table[:shared_with_group_id])) + .reorder(Arel::Nodes::Descending.new(group_group_link_table[:group_access])) + .first + + link&.group_access + end + def self.groups_including_descendants_by(group_ids) Gitlab::ObjectHierarchy .new(Group.where(id: group_ids)) diff --git a/app/models/group_group_link.rb b/app/models/group_group_link.rb new file mode 100644 index 00000000000..4b279b7af5b --- /dev/null +++ b/app/models/group_group_link.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class GroupGroupLink < ApplicationRecord + include Expirable + + belongs_to :shared_group, class_name: 'Group', foreign_key: :shared_group_id + belongs_to :shared_with_group, class_name: 'Group', foreign_key: :shared_with_group_id + + validates :shared_group, presence: true + validates :shared_group_id, uniqueness: { scope: [:shared_with_group_id], + message: _('The group has already been shared with this group') } + validates :shared_with_group, presence: true + validates :group_access, inclusion: { in: Gitlab::Access.values }, + presence: true + + def self.access_options + Gitlab::Access.options + end + + def self.default_access + Gitlab::Access::DEVELOPER + end +end diff --git a/app/services/groups/group_links/create_service.rb b/app/services/groups/group_links/create_service.rb new file mode 100644 index 00000000000..2ce53fcfe4a --- /dev/null +++ b/app/services/groups/group_links/create_service.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Groups + module GroupLinks + class CreateService < BaseService + def execute(shared_group) + unless group && shared_group && + can?(current_user, :admin_group, shared_group) && + can?(current_user, :read_group, group) + return error('Not Found', 404) + end + + link = GroupGroupLink.new( + shared_group: shared_group, + shared_with_group: group, + group_access: params[:shared_group_access], + expires_at: params[:expires_at] + ) + + if link.save + group.refresh_members_authorized_projects + success(link: link) + else + error(link.errors.full_messages.to_sentence, 409) + end + end + end + end +end diff --git a/app/views/ci/variables/_header.html.haml b/app/views/ci/variables/_header.html.haml index dbfa0a9e5a1..ce4dd5a4877 100644 --- a/app/views/ci/variables/_header.html.haml +++ b/app/views/ci/variables/_header.html.haml @@ -7,5 +7,5 @@ %button.btn.btn-default.js-settings-toggle{ type: 'button' } = expanded ? _('Collapse') : _('Expand') -%p.append-bottom-0 +%p = render "ci/variables/content" diff --git a/app/workers/authorized_projects_worker.rb b/app/workers/authorized_projects_worker.rb index 577c439f4a2..9492cfe217c 100644 --- a/app/workers/authorized_projects_worker.rb +++ b/app/workers/authorized_projects_worker.rb @@ -5,6 +5,7 @@ class AuthorizedProjectsWorker prepend WaitableWorker feature_category :authentication_and_authorization + latency_sensitive_worker! # This is a workaround for a Ruby 2.3.7 bug. rspec-mocks cannot restore the # visibility of prepended modules. See https://github.com/rspec/rspec-mocks/issues/1231 diff --git a/app/workers/build_finished_worker.rb b/app/workers/build_finished_worker.rb index e95b6b38d28..e61f37ddce1 100644 --- a/app/workers/build_finished_worker.rb +++ b/app/workers/build_finished_worker.rb @@ -5,6 +5,8 @@ class BuildFinishedWorker include PipelineQueue queue_namespace :pipeline_processing + latency_sensitive_worker! + worker_resource_boundary :cpu # rubocop: disable CodeReuse/ActiveRecord def perform(build_id) diff --git a/app/workers/build_hooks_worker.rb b/app/workers/build_hooks_worker.rb index 15b31acf3e5..fa55769e486 100644 --- a/app/workers/build_hooks_worker.rb +++ b/app/workers/build_hooks_worker.rb @@ -6,6 +6,7 @@ class BuildHooksWorker queue_namespace :pipeline_hooks feature_category :continuous_integration + latency_sensitive_worker! # rubocop: disable CodeReuse/ActiveRecord def perform(build_id) diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb index 6584fba4c65..6f75f403e6e 100644 --- a/app/workers/build_queue_worker.rb +++ b/app/workers/build_queue_worker.rb @@ -6,6 +6,8 @@ class BuildQueueWorker queue_namespace :pipeline_processing feature_category :continuous_integration + latency_sensitive_worker! + worker_resource_boundary :cpu # rubocop: disable CodeReuse/ActiveRecord def perform(build_id) diff --git a/app/workers/build_success_worker.rb b/app/workers/build_success_worker.rb index ac947f3cf38..b7dbd367fee 100644 --- a/app/workers/build_success_worker.rb +++ b/app/workers/build_success_worker.rb @@ -5,6 +5,7 @@ class BuildSuccessWorker include PipelineQueue queue_namespace :pipeline_processing + latency_sensitive_worker! # rubocop: disable CodeReuse/ActiveRecord def perform(build_id) diff --git a/app/workers/chat_notification_worker.rb b/app/workers/chat_notification_worker.rb index 3bc2edad62c..42a23cd472a 100644 --- a/app/workers/chat_notification_worker.rb +++ b/app/workers/chat_notification_worker.rb @@ -4,6 +4,11 @@ class ChatNotificationWorker include ApplicationWorker feature_category :chatops + latency_sensitive_worker! + # TODO: break this into multiple jobs + # as the `responder` uses external dependencies + # See https://gitlab.com/gitlab-com/gl-infra/scalability/issues/34 + # worker_has_external_dependencies! RESCHEDULE_INTERVAL = 2.seconds diff --git a/app/workers/ci/build_schedule_worker.rb b/app/workers/ci/build_schedule_worker.rb index f22ec4c7810..e34f16f46c2 100644 --- a/app/workers/ci/build_schedule_worker.rb +++ b/app/workers/ci/build_schedule_worker.rb @@ -7,6 +7,7 @@ module Ci queue_namespace :pipeline_processing feature_category :continuous_integration + worker_resource_boundary :cpu def perform(build_id) ::Ci::Build.find_by_id(build_id).try do |build| diff --git a/app/workers/cluster_install_app_worker.rb b/app/workers/cluster_install_app_worker.rb index 32e2ea7996c..0e075b295dd 100644 --- a/app/workers/cluster_install_app_worker.rb +++ b/app/workers/cluster_install_app_worker.rb @@ -5,6 +5,8 @@ class ClusterInstallAppWorker include ClusterQueue include ClusterApplications + worker_has_external_dependencies! + def perform(app_name, app_id) find_application(app_name, app_id) do |app| Clusters::Applications::InstallService.new(app).execute diff --git a/app/workers/cluster_patch_app_worker.rb b/app/workers/cluster_patch_app_worker.rb index 0549e81ed05..3f95a764567 100644 --- a/app/workers/cluster_patch_app_worker.rb +++ b/app/workers/cluster_patch_app_worker.rb @@ -5,6 +5,8 @@ class ClusterPatchAppWorker include ClusterQueue include ClusterApplications + worker_has_external_dependencies! + def perform(app_name, app_id) find_application(app_name, app_id) do |app| Clusters::Applications::PatchService.new(app).execute diff --git a/app/workers/cluster_project_configure_worker.rb b/app/workers/cluster_project_configure_worker.rb index ad2437a77e9..614029c2b5c 100644 --- a/app/workers/cluster_project_configure_worker.rb +++ b/app/workers/cluster_project_configure_worker.rb @@ -4,6 +4,8 @@ class ClusterProjectConfigureWorker include ApplicationWorker include ClusterQueue + worker_has_external_dependencies! + def perform(project_id) # Scheduled for removal in https://gitlab.com/gitlab-org/gitlab-foss/issues/59319 end diff --git a/app/workers/cluster_provision_worker.rb b/app/workers/cluster_provision_worker.rb index 59de7903c1c..375bcff4d3d 100644 --- a/app/workers/cluster_provision_worker.rb +++ b/app/workers/cluster_provision_worker.rb @@ -4,6 +4,8 @@ class ClusterProvisionWorker include ApplicationWorker include ClusterQueue + worker_has_external_dependencies! + def perform(cluster_id) Clusters::Cluster.find_by_id(cluster_id).try do |cluster| cluster.provider.try do |provider| diff --git a/app/workers/cluster_upgrade_app_worker.rb b/app/workers/cluster_upgrade_app_worker.rb index d1a538859b4..cd06f0a2224 100644 --- a/app/workers/cluster_upgrade_app_worker.rb +++ b/app/workers/cluster_upgrade_app_worker.rb @@ -5,6 +5,8 @@ class ClusterUpgradeAppWorker include ClusterQueue include ClusterApplications + worker_has_external_dependencies! + def perform(app_name, app_id) find_application(app_name, app_id) do |app| Clusters::Applications::UpgradeService.new(app).execute diff --git a/app/workers/cluster_wait_for_app_installation_worker.rb b/app/workers/cluster_wait_for_app_installation_worker.rb index e8d7e52f70f..7155dc6f835 100644 --- a/app/workers/cluster_wait_for_app_installation_worker.rb +++ b/app/workers/cluster_wait_for_app_installation_worker.rb @@ -8,6 +8,9 @@ class ClusterWaitForAppInstallationWorker INTERVAL = 10.seconds TIMEOUT = 20.minutes + worker_has_external_dependencies! + worker_resource_boundary :cpu + def perform(app_name, app_id) find_application(app_name, app_id) do |app| Clusters::Applications::CheckInstallationProgressService.new(app).execute diff --git a/app/workers/cluster_wait_for_ingress_ip_address_worker.rb b/app/workers/cluster_wait_for_ingress_ip_address_worker.rb index 6865384df44..14b1651cc72 100644 --- a/app/workers/cluster_wait_for_ingress_ip_address_worker.rb +++ b/app/workers/cluster_wait_for_ingress_ip_address_worker.rb @@ -5,6 +5,8 @@ class ClusterWaitForIngressIpAddressWorker include ClusterQueue include ClusterApplications + worker_has_external_dependencies! + def perform(app_name, app_id) find_application(app_name, app_id) do |app| Clusters::Applications::CheckIngressIpAddressService.new(app).execute diff --git a/app/workers/clusters/applications/uninstall_worker.rb b/app/workers/clusters/applications/uninstall_worker.rb index 85e8ecc4ad5..6180998c8d9 100644 --- a/app/workers/clusters/applications/uninstall_worker.rb +++ b/app/workers/clusters/applications/uninstall_worker.rb @@ -7,6 +7,8 @@ module Clusters include ClusterQueue include ClusterApplications + worker_has_external_dependencies! + def perform(app_name, app_id) find_application(app_name, app_id) do |app| Clusters::Applications::UninstallService.new(app).execute diff --git a/app/workers/clusters/applications/wait_for_uninstall_app_worker.rb b/app/workers/clusters/applications/wait_for_uninstall_app_worker.rb index 163c99d3c3c..7907aa8dfff 100644 --- a/app/workers/clusters/applications/wait_for_uninstall_app_worker.rb +++ b/app/workers/clusters/applications/wait_for_uninstall_app_worker.rb @@ -10,6 +10,9 @@ module Clusters INTERVAL = 10.seconds TIMEOUT = 20.minutes + worker_has_external_dependencies! + worker_resource_boundary :cpu + def perform(app_name, app_id) find_application(app_name, app_id) do |app| Clusters::Applications::CheckUninstallProgressService.new(app).execute diff --git a/app/workers/concerns/gitlab/github_import/object_importer.rb b/app/workers/concerns/gitlab/github_import/object_importer.rb index b856a9329dd..bd0b566658e 100644 --- a/app/workers/concerns/gitlab/github_import/object_importer.rb +++ b/app/workers/concerns/gitlab/github_import/object_importer.rb @@ -14,6 +14,7 @@ module Gitlab include NotifyUponDeath feature_category :importers + worker_has_external_dependencies! end # project - An instance of `Project` to import the data into. diff --git a/app/workers/create_pipeline_worker.rb b/app/workers/create_pipeline_worker.rb index 70412ffd095..a75cc643038 100644 --- a/app/workers/create_pipeline_worker.rb +++ b/app/workers/create_pipeline_worker.rb @@ -6,6 +6,8 @@ class CreatePipelineWorker queue_namespace :pipeline_creation feature_category :continuous_integration + latency_sensitive_worker! + worker_resource_boundary :cpu def perform(project_id, user_id, ref, source, params = {}) project = Project.find(project_id) diff --git a/app/workers/deployments/finished_worker.rb b/app/workers/deployments/finished_worker.rb index 79a1caccc92..90bbc193651 100644 --- a/app/workers/deployments/finished_worker.rb +++ b/app/workers/deployments/finished_worker.rb @@ -6,6 +6,7 @@ module Deployments queue_namespace :deployment feature_category :continuous_delivery + worker_resource_boundary :cpu def perform(deployment_id) Deployment.find_by_id(deployment_id).try(:execute_hooks) diff --git a/app/workers/deployments/success_worker.rb b/app/workers/deployments/success_worker.rb index f6520307186..4a29f1aef52 100644 --- a/app/workers/deployments/success_worker.rb +++ b/app/workers/deployments/success_worker.rb @@ -6,6 +6,7 @@ module Deployments queue_namespace :deployment feature_category :continuous_delivery + worker_resource_boundary :cpu def perform(deployment_id) Deployment.find_by_id(deployment_id).try do |deployment| diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index c82728be329..b56bf4ed833 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -4,6 +4,7 @@ class EmailReceiverWorker include ApplicationWorker feature_category :issue_tracking + latency_sensitive_worker! def perform(raw) return unless Gitlab::IncomingEmail.enabled? diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb index 2231c91a720..f523f5953e1 100644 --- a/app/workers/emails_on_push_worker.rb +++ b/app/workers/emails_on_push_worker.rb @@ -6,6 +6,8 @@ class EmailsOnPushWorker attr_reader :email, :skip_premailer feature_category :source_code_management + latency_sensitive_worker! + worker_resource_boundary :cpu def perform(project_id, recipients, push_data, options = {}) options.symbolize_keys! diff --git a/app/workers/expire_job_cache_worker.rb b/app/workers/expire_job_cache_worker.rb index b09d0a5d121..0363429587e 100644 --- a/app/workers/expire_job_cache_worker.rb +++ b/app/workers/expire_job_cache_worker.rb @@ -5,6 +5,7 @@ class ExpireJobCacheWorker include PipelineQueue queue_namespace :pipeline_cache + latency_sensitive_worker! # rubocop: disable CodeReuse/ActiveRecord def perform(job_id) diff --git a/app/workers/expire_pipeline_cache_worker.rb b/app/workers/expire_pipeline_cache_worker.rb index 78e68d7bf46..ab57c59ffda 100644 --- a/app/workers/expire_pipeline_cache_worker.rb +++ b/app/workers/expire_pipeline_cache_worker.rb @@ -5,6 +5,8 @@ class ExpirePipelineCacheWorker include PipelineQueue queue_namespace :pipeline_cache + latency_sensitive_worker! + worker_resource_boundary :cpu # rubocop: disable CodeReuse/ActiveRecord def perform(pipeline_id) diff --git a/app/workers/gitlab_shell_worker.rb b/app/workers/gitlab_shell_worker.rb index 9766331cf4b..5dcf901e041 100644 --- a/app/workers/gitlab_shell_worker.rb +++ b/app/workers/gitlab_shell_worker.rb @@ -5,6 +5,7 @@ class GitlabShellWorker include Gitlab::ShellAdapter feature_category :source_code_management + latency_sensitive_worker! def perform(action, *arg) gitlab_shell.__send__(action, *arg) # rubocop:disable GitlabSecurity/PublicSend diff --git a/app/workers/import_issues_csv_worker.rb b/app/workers/import_issues_csv_worker.rb index d9834320318..d2733dc5f56 100644 --- a/app/workers/import_issues_csv_worker.rb +++ b/app/workers/import_issues_csv_worker.rb @@ -4,6 +4,7 @@ class ImportIssuesCsvWorker include ApplicationWorker feature_category :issue_tracking + worker_resource_boundary :cpu sidekiq_retries_exhausted do |job| Upload.find(job['args'][2]).destroy diff --git a/app/workers/mail_scheduler/notification_service_worker.rb b/app/workers/mail_scheduler/notification_service_worker.rb index 0d06dab3b2e..4130ce25878 100644 --- a/app/workers/mail_scheduler/notification_service_worker.rb +++ b/app/workers/mail_scheduler/notification_service_worker.rb @@ -8,6 +8,7 @@ module MailScheduler include MailSchedulerQueue feature_category :issue_tracking + worker_resource_boundary :cpu def perform(meth, *args) check_arguments!(args) diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index 70b909afea8..ed88c57e8d4 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -4,6 +4,7 @@ class MergeWorker include ApplicationWorker feature_category :source_code_management + latency_sensitive_worker! def perform(merge_request_id, current_user_id, params) params = params.with_indifferent_access diff --git a/app/workers/namespaces/prune_aggregation_schedules_worker.rb b/app/workers/namespaces/prune_aggregation_schedules_worker.rb index 16259ffbfa6..9a5f533fe9a 100644 --- a/app/workers/namespaces/prune_aggregation_schedules_worker.rb +++ b/app/workers/namespaces/prune_aggregation_schedules_worker.rb @@ -6,6 +6,7 @@ module Namespaces include CronjobQueue feature_category :source_code_management + worker_resource_boundary :cpu # Worker to prune pending rows on Namespace::AggregationSchedule # It's scheduled to run once a day at 1:05am. diff --git a/app/workers/new_issue_worker.rb b/app/workers/new_issue_worker.rb index 1b0fec597e7..af9ca332d3c 100644 --- a/app/workers/new_issue_worker.rb +++ b/app/workers/new_issue_worker.rb @@ -5,6 +5,8 @@ class NewIssueWorker include NewIssuable feature_category :issue_tracking + latency_sensitive_worker! + worker_resource_boundary :cpu def perform(issue_id, user_id) return unless objects_found?(issue_id, user_id) diff --git a/app/workers/new_merge_request_worker.rb b/app/workers/new_merge_request_worker.rb index 0a5b2f86331..aa3f85c157b 100644 --- a/app/workers/new_merge_request_worker.rb +++ b/app/workers/new_merge_request_worker.rb @@ -5,6 +5,8 @@ class NewMergeRequestWorker include NewIssuable feature_category :source_code_management + latency_sensitive_worker! + worker_resource_boundary :cpu def perform(merge_request_id, user_id) return unless objects_found?(merge_request_id, user_id) diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb index d0d2a563738..2a5988a7e32 100644 --- a/app/workers/new_note_worker.rb +++ b/app/workers/new_note_worker.rb @@ -4,6 +4,8 @@ class NewNoteWorker include ApplicationWorker feature_category :issue_tracking + latency_sensitive_worker! + worker_resource_boundary :cpu # Keep extra parameter to preserve backwards compatibility with # old `NewNoteWorker` jobs (can remove later) diff --git a/app/workers/object_pool/join_worker.rb b/app/workers/object_pool/join_worker.rb index 9c5161fd55a..ddd002eabb8 100644 --- a/app/workers/object_pool/join_worker.rb +++ b/app/workers/object_pool/join_worker.rb @@ -5,6 +5,8 @@ module ObjectPool include ApplicationWorker include ObjectPoolQueue + worker_resource_boundary :cpu + # The use of pool id is deprecated. Keeping the argument allows old jobs to # still be performed. def perform(_pool_id, project_id) diff --git a/app/workers/pages_domain_removal_cron_worker.rb b/app/workers/pages_domain_removal_cron_worker.rb index 25e747c78d0..b1506831056 100644 --- a/app/workers/pages_domain_removal_cron_worker.rb +++ b/app/workers/pages_domain_removal_cron_worker.rb @@ -5,6 +5,7 @@ class PagesDomainRemovalCronWorker include CronjobQueue feature_category :pages + worker_resource_boundary :cpu def perform PagesDomain.for_removal.find_each do |domain| diff --git a/app/workers/pipeline_hooks_worker.rb b/app/workers/pipeline_hooks_worker.rb index eae1115e60c..04abc9c88fd 100644 --- a/app/workers/pipeline_hooks_worker.rb +++ b/app/workers/pipeline_hooks_worker.rb @@ -5,6 +5,8 @@ class PipelineHooksWorker include PipelineQueue queue_namespace :pipeline_hooks + latency_sensitive_worker! + worker_resource_boundary :cpu # rubocop: disable CodeReuse/ActiveRecord def perform(pipeline_id) diff --git a/app/workers/pipeline_metrics_worker.rb b/app/workers/pipeline_metrics_worker.rb index 0ddad43b8d5..3830522aaa1 100644 --- a/app/workers/pipeline_metrics_worker.rb +++ b/app/workers/pipeline_metrics_worker.rb @@ -4,6 +4,8 @@ class PipelineMetricsWorker include ApplicationWorker include PipelineQueue + latency_sensitive_worker! + # rubocop: disable CodeReuse/ActiveRecord def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline| diff --git a/app/workers/pipeline_notification_worker.rb b/app/workers/pipeline_notification_worker.rb index e4a18573d20..62ecbc8a047 100644 --- a/app/workers/pipeline_notification_worker.rb +++ b/app/workers/pipeline_notification_worker.rb @@ -4,6 +4,9 @@ class PipelineNotificationWorker include ApplicationWorker include PipelineQueue + latency_sensitive_worker! + worker_resource_boundary :cpu + # rubocop: disable CodeReuse/ActiveRecord def perform(pipeline_id, recipients = nil) pipeline = Ci::Pipeline.find_by(id: pipeline_id) diff --git a/app/workers/pipeline_process_worker.rb b/app/workers/pipeline_process_worker.rb index 96f3725dbbe..2a36ab992e9 100644 --- a/app/workers/pipeline_process_worker.rb +++ b/app/workers/pipeline_process_worker.rb @@ -6,6 +6,7 @@ class PipelineProcessWorker queue_namespace :pipeline_processing feature_category :continuous_integration + latency_sensitive_worker! # rubocop: disable CodeReuse/ActiveRecord def perform(pipeline_id, build_ids = nil) diff --git a/app/workers/pipeline_schedule_worker.rb b/app/workers/pipeline_schedule_worker.rb index f500ea08353..19c3c5fcc2f 100644 --- a/app/workers/pipeline_schedule_worker.rb +++ b/app/workers/pipeline_schedule_worker.rb @@ -5,6 +5,7 @@ class PipelineScheduleWorker include CronjobQueue feature_category :continuous_integration + worker_resource_boundary :cpu def perform Ci::PipelineSchedule.runnable_schedules.preloaded.find_in_batches do |schedules| diff --git a/app/workers/pipeline_success_worker.rb b/app/workers/pipeline_success_worker.rb index 666331e6cd4..5c24f00e0c3 100644 --- a/app/workers/pipeline_success_worker.rb +++ b/app/workers/pipeline_success_worker.rb @@ -5,6 +5,7 @@ class PipelineSuccessWorker include PipelineQueue queue_namespace :pipeline_processing + latency_sensitive_worker! def perform(pipeline_id) # no-op diff --git a/app/workers/pipeline_update_worker.rb b/app/workers/pipeline_update_worker.rb index 13a748e1551..5b742461f7a 100644 --- a/app/workers/pipeline_update_worker.rb +++ b/app/workers/pipeline_update_worker.rb @@ -5,6 +5,7 @@ class PipelineUpdateWorker include PipelineQueue queue_namespace :pipeline_processing + latency_sensitive_worker! # rubocop: disable CodeReuse/ActiveRecord def perform(pipeline_id) diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index a3bc7e5b9c9..334a98a0017 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -4,6 +4,8 @@ class PostReceive include ApplicationWorker feature_category :source_code_management + latency_sensitive_worker! + worker_resource_boundary :cpu def perform(gl_repository, identifier, changes, push_options = {}) project, repo_type = Gitlab::GlRepository.parse(gl_repository) diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb index 1e4561fc6ea..8b4d66ae493 100644 --- a/app/workers/process_commit_worker.rb +++ b/app/workers/process_commit_worker.rb @@ -11,6 +11,7 @@ class ProcessCommitWorker include ApplicationWorker feature_category :source_code_management + latency_sensitive_worker! # project_id - The ID of the project this commit belongs to. # user_id - The ID of the user that pushed the commit. diff --git a/app/workers/project_cache_worker.rb b/app/workers/project_cache_worker.rb index 57a01c0dd8e..ae1d57aa124 100644 --- a/app/workers/project_cache_worker.rb +++ b/app/workers/project_cache_worker.rb @@ -3,6 +3,9 @@ # Worker for updating any project specific caches. class ProjectCacheWorker include ApplicationWorker + + latency_sensitive_worker! + LEASE_TIMEOUT = 15.minutes.to_i feature_category :source_code_management diff --git a/app/workers/project_export_worker.rb b/app/workers/project_export_worker.rb index bbcf3b72718..11f3fed82cd 100644 --- a/app/workers/project_export_worker.rb +++ b/app/workers/project_export_worker.rb @@ -6,6 +6,7 @@ class ProjectExportWorker sidekiq_options retry: 3 feature_category :source_code_management + worker_resource_boundary :memory def perform(current_user_id, project_id, after_export_strategy = {}, params = {}) current_user = User.find(current_user_id) diff --git a/app/workers/project_service_worker.rb b/app/workers/project_service_worker.rb index 8041404fc71..38a2a7414a5 100644 --- a/app/workers/project_service_worker.rb +++ b/app/workers/project_service_worker.rb @@ -5,6 +5,7 @@ class ProjectServiceWorker sidekiq_options dead: false feature_category :integrations + worker_has_external_dependencies! def perform(hook_id, data) data = data.with_indifferent_access diff --git a/app/workers/reactive_caching_worker.rb b/app/workers/reactive_caching_worker.rb index af4a3def062..f3a83e0e8d4 100644 --- a/app/workers/reactive_caching_worker.rb +++ b/app/workers/reactive_caching_worker.rb @@ -5,6 +5,14 @@ class ReactiveCachingWorker feature_category_not_owned! + # TODO: The reactive caching worker should be split into + # two different workers, one for latency_sensitive jobs without external dependencies + # and another worker without latency_sensitivity, but with external dependencies + # https://gitlab.com/gitlab-com/gl-infra/scalability/issues/34 + # This worker should also have `worker_has_external_dependencies!` enabled + latency_sensitive_worker! + worker_resource_boundary :cpu + def perform(class_name, id, *args) klass = begin class_name.constantize diff --git a/app/workers/remove_expired_members_worker.rb b/app/workers/remove_expired_members_worker.rb index 75f06fd9f6b..bf209fcec9f 100644 --- a/app/workers/remove_expired_members_worker.rb +++ b/app/workers/remove_expired_members_worker.rb @@ -5,6 +5,7 @@ class RemoveExpiredMembersWorker include CronjobQueue feature_category :authentication_and_authorization + worker_resource_boundary :cpu def perform Member.expired.find_each do |member| diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index bc2d0366fdd..15677fb0a95 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -7,6 +7,7 @@ class RepositoryImportWorker include ProjectImportOptions feature_category :importers + worker_has_external_dependencies! # technical debt: https://gitlab.com/gitlab-org/gitlab/issues/33991 sidekiq_options memory_killer_memory_growth_kb: ENV.fetch('MEMORY_KILLER_REPOSITORY_IMPORT_WORKER_MEMORY_GROWTH_KB', 50).to_i diff --git a/app/workers/repository_update_remote_mirror_worker.rb b/app/workers/repository_update_remote_mirror_worker.rb index b4d96546fa4..d1dec4cb732 100644 --- a/app/workers/repository_update_remote_mirror_worker.rb +++ b/app/workers/repository_update_remote_mirror_worker.rb @@ -6,6 +6,8 @@ class RepositoryUpdateRemoteMirrorWorker include ApplicationWorker include Gitlab::ExclusiveLeaseHelpers + worker_has_external_dependencies! + sidekiq_options retry: 3, dead: false feature_category :source_code_management diff --git a/app/workers/stage_update_worker.rb b/app/workers/stage_update_worker.rb index ea587789d03..de2454128f6 100644 --- a/app/workers/stage_update_worker.rb +++ b/app/workers/stage_update_worker.rb @@ -5,6 +5,7 @@ class StageUpdateWorker include PipelineQueue queue_namespace :pipeline_processing + latency_sensitive_worker! # rubocop: disable CodeReuse/ActiveRecord def perform(stage_id) diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb index 971edb1f14f..5a248ab1137 100644 --- a/app/workers/stuck_ci_jobs_worker.rb +++ b/app/workers/stuck_ci_jobs_worker.rb @@ -5,6 +5,7 @@ class StuckCiJobsWorker include CronjobQueue feature_category :continuous_integration + worker_resource_boundary :cpu EXCLUSIVE_LEASE_KEY = 'stuck_ci_builds_worker_lease' diff --git a/app/workers/stuck_import_jobs_worker.rb b/app/workers/stuck_import_jobs_worker.rb index 4993cd1220c..d9a9a613ca9 100644 --- a/app/workers/stuck_import_jobs_worker.rb +++ b/app/workers/stuck_import_jobs_worker.rb @@ -5,6 +5,7 @@ class StuckImportJobsWorker include CronjobQueue feature_category :importers + worker_resource_boundary :cpu IMPORT_JOBS_EXPIRATION = 15.hours.to_i diff --git a/app/workers/update_head_pipeline_for_merge_request_worker.rb b/app/workers/update_head_pipeline_for_merge_request_worker.rb index 77859abfea4..e069b16eb90 100644 --- a/app/workers/update_head_pipeline_for_merge_request_worker.rb +++ b/app/workers/update_head_pipeline_for_merge_request_worker.rb @@ -6,6 +6,8 @@ class UpdateHeadPipelineForMergeRequestWorker queue_namespace :pipeline_processing feature_category :continuous_integration + latency_sensitive_worker! + worker_resource_boundary :cpu def perform(merge_request_id) MergeRequest.find_by_id(merge_request_id).try do |merge_request| diff --git a/app/workers/update_merge_requests_worker.rb b/app/workers/update_merge_requests_worker.rb index 8e1703cdd0b..acb95353983 100644 --- a/app/workers/update_merge_requests_worker.rb +++ b/app/workers/update_merge_requests_worker.rb @@ -4,6 +4,8 @@ class UpdateMergeRequestsWorker include ApplicationWorker feature_category :source_code_management + latency_sensitive_worker! + worker_resource_boundary :cpu LOG_TIME_THRESHOLD = 90 # seconds diff --git a/app/workers/wait_for_cluster_creation_worker.rb b/app/workers/wait_for_cluster_creation_worker.rb index 8aa1d9290fd..770417398cb 100644 --- a/app/workers/wait_for_cluster_creation_worker.rb +++ b/app/workers/wait_for_cluster_creation_worker.rb @@ -4,6 +4,8 @@ class WaitForClusterCreationWorker include ApplicationWorker include ClusterQueue + worker_has_external_dependencies! + def perform(cluster_id) Clusters::Cluster.find_by_id(cluster_id).try do |cluster| cluster.provider.try do |provider| diff --git a/app/workers/web_hook_worker.rb b/app/workers/web_hook_worker.rb index fd7ca93683e..c3fa3162c14 100644 --- a/app/workers/web_hook_worker.rb +++ b/app/workers/web_hook_worker.rb @@ -4,6 +4,8 @@ class WebHookWorker include ApplicationWorker feature_category :integrations + worker_has_external_dependencies! + sidekiq_options retry: 4, dead: false def perform(hook_id, data, hook_name) diff --git a/changelogs/unreleased/32935-preventing-accidental-project-deletion-db-changes.yml b/changelogs/unreleased/32935-preventing-accidental-project-deletion-db-changes.yml new file mode 100644 index 00000000000..15b9f0d55cb --- /dev/null +++ b/changelogs/unreleased/32935-preventing-accidental-project-deletion-db-changes.yml @@ -0,0 +1,5 @@ +--- +title: Add migrations and changes for soft-delete for projects +merge_request: 18791 +author: +type: added diff --git a/changelogs/unreleased/33054-share_groups_with_groups.yml b/changelogs/unreleased/33054-share_groups_with_groups.yml new file mode 100644 index 00000000000..0a1f9e5cb3b --- /dev/null +++ b/changelogs/unreleased/33054-share_groups_with_groups.yml @@ -0,0 +1,5 @@ +--- +title: Share groups with groups +merge_request: 17117 +author: +type: added diff --git a/changelogs/unreleased/5366-display-anomaly-deviation-boundaries-on-dashboard-ce.yml b/changelogs/unreleased/5366-display-anomaly-deviation-boundaries-on-dashboard-ce.yml new file mode 100644 index 00000000000..08b85156259 --- /dev/null +++ b/changelogs/unreleased/5366-display-anomaly-deviation-boundaries-on-dashboard-ce.yml @@ -0,0 +1,5 @@ +--- +title: Added new chart component to display an anomaly boundary +merge_request: 16530 +author: +type: added diff --git a/changelogs/unreleased/add-missing-bottom-padding-in-settings.yml b/changelogs/unreleased/add-missing-bottom-padding-in-settings.yml new file mode 100644 index 00000000000..b87f83ce33a --- /dev/null +++ b/changelogs/unreleased/add-missing-bottom-padding-in-settings.yml @@ -0,0 +1,5 @@ +--- +title: Add missing bottom padding in CI/CD settings +merge_request: 19284 +author: George Tsiolis +type: fixed diff --git a/changelogs/unreleased/an-mark-jobs-as-latency-sensitive.yml b/changelogs/unreleased/an-mark-jobs-as-latency-sensitive.yml new file mode 100644 index 00000000000..0ed019b8500 --- /dev/null +++ b/changelogs/unreleased/an-mark-jobs-as-latency-sensitive.yml @@ -0,0 +1,5 @@ +--- +title: Attribute Sidekiq workers according to their workloads +merge_request: 18066 +author: +type: other diff --git a/changelogs/unreleased/security-open-redirect-internalredirect.yml b/changelogs/unreleased/security-open-redirect-internalredirect.yml deleted file mode 100644 index 5ac65a4b355..00000000000 --- a/changelogs/unreleased/security-open-redirect-internalredirect.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fixes a Open Redirect issue in `InternalRedirect`. -merge_request: -author: -type: security diff --git a/changelogs/unreleased/security-remove-deploy-access-levels-on-project-group-link-deletion.yml b/changelogs/unreleased/security-remove-deploy-access-levels-on-project-group-link-deletion.yml new file mode 100644 index 00000000000..b570d182a53 --- /dev/null +++ b/changelogs/unreleased/security-remove-deploy-access-levels-on-project-group-link-deletion.yml @@ -0,0 +1,5 @@ +--- +title: Remove deploy access level when project/group link is deleted +merge_request: +author: +type: security diff --git a/config/routes/group.rb b/config/routes/group.rb index 093cde64c85..437c80b8c92 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -62,6 +62,8 @@ constraints(::Constraints::GroupUrlConstrainer.new) do delete :leave, on: :collection end + resources :group_links, only: [:index, :create, :update, :destroy], constraints: { id: /\d+/ } + resources :uploads, only: [:create] do collection do get ":secret/:filename", action: :show, as: :show, constraints: { filename: %r{[^/]+} } diff --git a/db/migrate/20180902070406_create_group_group_links.rb b/db/migrate/20180902070406_create_group_group_links.rb new file mode 100644 index 00000000000..95fed0ebf96 --- /dev/null +++ b/db/migrate/20180902070406_create_group_group_links.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +class CreateGroupGroupLinks < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + create_table :group_group_links do |t| + t.timestamps_with_timezone null: false + + t.references :shared_group, null: false, + index: false, + foreign_key: { on_delete: :cascade, + to_table: :namespaces } + t.references :shared_with_group, null: false, + foreign_key: { on_delete: :cascade, + to_table: :namespaces } + t.date :expires_at + t.index [:shared_group_id, :shared_with_group_id], + { unique: true, + name: 'index_group_group_links_on_shared_group_and_shared_with_group' } + t.integer :group_access, { limit: 2, + default: 30, # Gitlab::Access::DEVELOPER + null: false } + end + end + + def down + drop_table :group_group_links + end +end diff --git a/db/migrate/20191003161031_add_mark_for_deletion_to_projects.rb b/db/migrate/20191003161031_add_mark_for_deletion_to_projects.rb new file mode 100644 index 00000000000..86d581a4383 --- /dev/null +++ b/db/migrate/20191003161031_add_mark_for_deletion_to_projects.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddMarkForDeletionToProjects < ActiveRecord::Migration[5.2] + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def change + add_column :projects, :marked_for_deletion_at, :date + add_column :projects, :marked_for_deletion_by_user_id, :integer + end +end diff --git a/db/migrate/20191003161032_add_mark_for_deletion_indexes_to_projects.rb b/db/migrate/20191003161032_add_mark_for_deletion_indexes_to_projects.rb new file mode 100644 index 00000000000..d6ef6509fff --- /dev/null +++ b/db/migrate/20191003161032_add_mark_for_deletion_indexes_to_projects.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddMarkForDeletionIndexesToProjects < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :projects, :users, column: :marked_for_deletion_by_user_id, on_delete: :nullify + add_concurrent_index :projects, :marked_for_deletion_by_user_id, where: 'marked_for_deletion_by_user_id IS NOT NULL' + end + + def down + remove_foreign_key_if_exists :projects, column: :marked_for_deletion_by_user_id + remove_concurrent_index :projects, :marked_for_deletion_by_user_id + end +end diff --git a/db/migrate/20191011084019_add_project_deletion_adjourned_period_to_application_settings.rb b/db/migrate/20191011084019_add_project_deletion_adjourned_period_to_application_settings.rb new file mode 100644 index 00000000000..79546e33253 --- /dev/null +++ b/db/migrate/20191011084019_add_project_deletion_adjourned_period_to_application_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddProjectDeletionAdjournedPeriodToApplicationSettings < ActiveRecord::Migration[5.2] + DOWNTIME = false + + DEFAULT_NUMBER_OF_DAYS_BEFORE_REMOVAL = 7 + + def change + add_column :application_settings, :deletion_adjourned_period, :integer, default: DEFAULT_NUMBER_OF_DAYS_BEFORE_REMOVAL, null: false + end +end diff --git a/db/post_migrate/20190926180443_schedule_epic_issues_after_epics_move.rb b/db/post_migrate/20190926180443_schedule_epic_issues_after_epics_move.rb new file mode 100644 index 00000000000..4e5b05ce363 --- /dev/null +++ b/db/post_migrate/20190926180443_schedule_epic_issues_after_epics_move.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class ScheduleEpicIssuesAfterEpicsMove < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INTERVAL = 5.minutes.to_i + BATCH_SIZE = 100 + MIGRATION = 'MoveEpicIssuesAfterEpics' + + disable_ddl_transaction! + + class Epic < ActiveRecord::Base + self.table_name = 'epics' + + include ::EachBatch + end + + def up + return unless ::Gitlab.ee? + + Epic.each_batch(of: BATCH_SIZE) do |batch, index| + range = batch.pluck('MIN(id)', 'MAX(id)').first + delay = index * interval + BackgroundMigrationWorker.perform_in(delay, MIGRATION, *range) + end + end + + def down + # no need + end +end diff --git a/db/schema.rb b/db/schema.rb index c4a541824c8..df56b84d61b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -343,6 +343,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do t.string "custom_http_clone_url_root", limit: 511 t.boolean "pendo_enabled", default: false, null: false t.string "pendo_url", limit: 255 + t.integer "deletion_adjourned_period", default: 7, null: false t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id" t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id" t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id" @@ -1820,6 +1821,17 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do t.index ["key", "value"], name: "index_group_custom_attributes_on_key_and_value" end + create_table "group_group_links", force: :cascade do |t| + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.bigint "shared_group_id", null: false + t.bigint "shared_with_group_id", null: false + t.date "expires_at" + t.integer "group_access", limit: 2, default: 30, null: false + t.index ["shared_group_id", "shared_with_group_id"], name: "index_group_group_links_on_shared_group_and_shared_with_group", unique: true + t.index ["shared_with_group_id"], name: "index_group_group_links_on_shared_with_group_id" + end + create_table "historical_data", id: :serial, force: :cascade do |t| t.date "date", null: false t.integer "active_user_count" @@ -3062,6 +3074,8 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do t.integer "max_artifacts_size" t.string "pull_mirror_branch_prefix", limit: 50 t.boolean "remove_source_branch_after_merge" + t.date "marked_for_deletion_at" + t.integer "marked_for_deletion_by_user_id" t.index "lower((name)::text)", name: "index_projects_on_lower_name" t.index ["archived", "pending_delete", "merge_requests_require_code_owner_approval"], name: "projects_requiring_code_owner_approval", where: "((pending_delete = false) AND (archived = false) AND (merge_requests_require_code_owner_approval = true))" t.index ["created_at"], name: "index_projects_on_created_at" @@ -3074,6 +3088,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do t.index ["last_repository_check_at"], name: "index_projects_on_last_repository_check_at", where: "(last_repository_check_at IS NOT NULL)" t.index ["last_repository_check_failed"], name: "index_projects_on_last_repository_check_failed" t.index ["last_repository_updated_at"], name: "index_projects_on_last_repository_updated_at" + t.index ["marked_for_deletion_by_user_id"], name: "index_projects_on_marked_for_deletion_by_user_id", where: "(marked_for_deletion_by_user_id IS NOT NULL)" t.index ["mirror_last_successful_update_at"], name: "index_projects_on_mirror_last_successful_update_at" t.index ["mirror_user_id"], name: "index_projects_on_mirror_user_id" t.index ["name"], name: "index_projects_on_name_trigram", opclass: :gin_trgm_ops, using: :gin @@ -4220,6 +4235,8 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do add_foreign_key "gpg_signatures", "projects", on_delete: :cascade add_foreign_key "grafana_integrations", "projects", on_delete: :cascade add_foreign_key "group_custom_attributes", "namespaces", column: "group_id", on_delete: :cascade + add_foreign_key "group_group_links", "namespaces", column: "shared_group_id", on_delete: :cascade + add_foreign_key "group_group_links", "namespaces", column: "shared_with_group_id", on_delete: :cascade add_foreign_key "identities", "saml_providers", name: "fk_aade90f0fc", on_delete: :cascade add_foreign_key "import_export_uploads", "projects", on_delete: :cascade add_foreign_key "index_statuses", "projects", name: "fk_74b2492545", on_delete: :cascade @@ -4343,6 +4360,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do add_foreign_key "project_statistics", "projects", on_delete: :cascade add_foreign_key "project_tracing_settings", "projects", on_delete: :cascade add_foreign_key "projects", "pool_repositories", name: "fk_6e5c14658a", on_delete: :nullify + add_foreign_key "projects", "users", column: "marked_for_deletion_by_user_id", name: "fk_25d8780d11", on_delete: :nullify add_foreign_key "prometheus_alert_events", "projects", on_delete: :cascade add_foreign_key "prometheus_alert_events", "prometheus_alerts", on_delete: :cascade add_foreign_key "prometheus_alerts", "environments", on_delete: :cascade diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 421b70bd2db..a19f40d1824 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -368,7 +368,7 @@ Enterprise Edition instance. This has some implications: - [Background migrations](background_migrations.md) run in Sidekiq, and should only be done for migrations that would take an extreme amount of time at GitLab.com scale. -1. **Sidekiq workers** [cannot change in a backwards-incompatible way](sidekiq_style_guide.md#removing-or-renaming-queues): +1. **Sidekiq workers** [cannot change in a backwards-incompatible way](sidekiq_style_guide.md#sidekiq-compatibility-across-updates): 1. Sidekiq queues are not drained before a deploy happens, so there will be workers in the queue from the previous version of GitLab. 1. If you need to change a method signature, try to do so across two releases, diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index d52a3e652e3..e433691c1ed 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -61,6 +61,168 @@ the extra jobs will take resources away from jobs from workers that were already there, if the resources available to the Sidekiq process handling the namespace are not adjusted appropriately. +## Latency Sensitive Jobs + +If a large number of background jobs get scheduled at once, queueing of jobs may +occur while jobs wait for a worker node to be become available. This is normal +and gives the system resilience by allowing it to gracefully handle spikes in +traffic. Some jobs, however, are more sensitive to latency than others. Examples +of these jobs include: + +1. A job which updates a merge request following a push to a branch. +1. A job which invalidates a cache of known branches for a project after a push + to the branch. +1. A job which recalculates the groups and projects a user can see after a + change in permissions. +1. A job which updates the status of a CI pipeline after a state change to a job + in the pipeline. + +When these jobs are delayed, the user may perceive the delay as a bug: for +example, they may push a branch and then attempt to create a merge request for +that branch, but be told in the UI that the branch does not exist. We deem these +jobs to be `latency_sensitive`. + +Extra effort is made to ensure that these jobs are started within a very short +period of time after being scheduled. However, in order to ensure throughput, +these jobs also have very strict execution duration requirements: + +1. The median job execution time should be less than 1 second. +1. 99% of jobs should complete within 10 seconds. + +If a worker cannot meet these expectations, then it cannot be treated as a +`latency_sensitive` worker: consider redesigning the worker, or splitting the +work between two different workers, one with `latency_sensitive` code that +executes quickly, and the other with non-`latency_sensitive`, which has no +execution latency requirements (but also has lower scheduling targets). + +This can be summed up in the following table: + +| **Latency Sensitivity** | **Queue Scheduling Target** | **Execution Latency Requirement** | +|-------------------------|-----------------------------|-------------------------------------| +| Not `latency_sensitive` | 1 minute | Maximum run time of 1 hour | +| `latency_sensitive` | 100 milliseconds | p50 of 1 second, p99 of 10 seconds | + +To mark a worker as being `latency_sensitive`, use the +`latency_sensitive_worker!` attribute, as shown in this example: + +```ruby +class LatencySensitiveWorker + include ApplicationWorker + + latency_sensitive_worker! + + # ... +end +``` + +## Jobs with External Dependencies + +Most background jobs in the GitLab application communicate with other GitLab +services, eg Postgres, Redis, Gitaly and Object Storage. These are considered +to be "internal" dependencies for a job. + +However, some jobs will be dependent on external services in order to complete +successfully. Some examples include: + +1. Jobs which call web-hooks configured by a user. +1. Jobs which deploy an application to a k8s cluster configured by a user. + +These jobs have "external dependencies". This is important for the operation of +the background processing cluster in several ways: + +1. Most external dependencies (such as web-hooks) do not provide SLOs, and + therefore we cannot guarantee the execution latencies on these jobs. Since we + cannot guarantee execution latency, we cannot ensure throughput and + therefore, in high-traffic environments, we need to ensure that jobs with + external dependencies are separated from `latency_sensitive` jobs, to ensure + throughput on those queues. +1. Errors in jobs with external dependencies have higher alerting thresholds as + there is a likelihood that the cause of the error is external. + +```ruby +class ExternalDependencyWorker + include ApplicationWorker + + # Declares that this worker depends on + # third-party, external services in order + # to complete successfully + worker_has_external_dependencies! + + # ... +end +``` + +NOTE: **Note:** Note that a job cannot be both latency sensitive and have +external dependencies. + +## CPU-bound and Memory-bound Workers + +Workers that are constrained by CPU or memory resource limitations should be +annotated with the `worker_resource_boundary` method. + +Most workers tend to spend most of their time blocked, wait on network responses +from other services such as Redis, Postgres and Gitaly. Since Sidekiq is a +multithreaded environment, these jobs can be scheduled with high concurrency. + +Some workers, however, spend large amounts of time _on-cpu_ running logic in +Ruby. Ruby MRI does not support true multithreading - it relies on the +[GIL](https://thoughtbot.com/blog/untangling-ruby-threads#the-global-interpreter-lock) +to greatly simplify application development by only allowing one section of Ruby +code in a process to run at a time, no matter how many cores the machine +hosting the process has. For IO bound workers, this is not a problem, since most +of the threads are blocked in underlying libraries (which are outside of the +GIL). + +If many threads are attempting to run Ruby code simultaneously, this will lead +to contention on the GIL which will have the affect of slowing down all +processes. + +In high-traffic environments, knowing that a worker is CPU-bound allows us to +run it on a different fleet with lower concurrency. This ensures optimal +performance. + +Likewise, if a worker uses large amounts of memory, we can run these on a +bespoke low concurrency, high memory fleet. + +Note that Memory-bound workers create heavy GC workloads, with pauses of +10-50ms. This will have an impact on the latency requirements for the +worker. For this reason, `memory` bound, `latency_sensitive` jobs are not +permitted and will fail CI. In general, `memory` bound workers are +discouraged, and alternative approaches to processing the work should be +considered. + +## Declaring a Job as CPU-bound + +This example shows how to declare a job as being CPU-bound. + +```ruby +class CPUIntensiveWorker + include ApplicationWorker + + # Declares that this worker will perform a lot of + # calculations on-CPU. + worker_resource_boundary :cpu + + # ... +end +``` + +## Determining whether a worker is CPU-bound + +We use the following approach to determine whether a worker is CPU-bound: + +- In the sidekiq structured JSON logs, aggregate the worker `duration` and + `cpu_s` fields. +- `duration` refers to the total job execution duration, in seconds +- `cpu_s` is derived from the + [`Process::CLOCK_THREAD_CPUTIME_ID`](https://www.rubydoc.info/stdlib/core/Process:clock_gettime) + counter, and is a measure of time spent by the job on-CPU. +- Divide `cpu_s` by `duration` to get the percentage time spend on-CPU. +- If this ratio exceeds 33%, the worker is considered CPU-bound and should be + annotated as such. +- Note that these values should not be used over small sample sizes, but + rather over fairly large aggregates. + ## Feature Categorization Each Sidekiq worker, or one of its ancestor classes, must declare a @@ -74,7 +236,7 @@ The declaration uses the `feature_category` class method, as shown below. class SomeScheduledTaskWorker include ApplicationWorker - # Declares that this feature is part of the + # Declares that this worker is part of the # `continuous_integration` feature category feature_category :continuous_integration @@ -88,11 +250,11 @@ source](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml ### Updating `config/feature_categories.yml` -Occassionally new features will be added to GitLab stages. When this occurs, you +Occasionally new features will be added to GitLab stages. When this occurs, you can automatically update `config/feature_categories.yml` by running `scripts/update-feature-categories`. This script will fetch and parse [`stages.yml`](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml) -and generare a new version of the file, which needs to be checked into source control. +and generate a new version of the file, which needs to be checked into source control. ### Excluding Sidekiq workers from feature categorization @@ -116,9 +278,63 @@ end Each Sidekiq worker must be tested using RSpec, just like any other class. These tests should be placed in `spec/workers`. -## Removing or renaming queues +## Sidekiq Compatibility across Updates + +Keep in mind that the arguments for a Sidekiq job are stored in a queue while it +is scheduled for execution. During a online update, this could lead to several +possible situations: + +1. An older version of the application publishes a job, which is executed by an + upgraded Sidekiq node. +1. A job is queued before an upgrade, but executed after an upgrade. +1. A job is queued by a node running the newer version of the application, but + executed on a node running an older version of the application. + +### Changing the arguments for a worker + +Jobs need to be backwards- and forwards-compatible between consecutive versions +of the application. + +This can be done by following this process: + +1. **Do not remove arguments from the `perform` function.**. Instead, use the + following approach + 1. Provide a default value (usually `nil`) and use a comment to mark the + argument as deprecated + 1. Stop using the argument in `perform_async`. + 1. Ignore the value in the worker class, but do not remove it until the next + major release. + +### Removing workers + +Try to avoid removing workers and their queues in minor and patch +releases. -Try to avoid renaming or removing workers and their queues in minor and patch releases. During online update instance can have pending jobs and removing the queue can lead to those jobs being stuck forever. If you can't write migration for those -Sidekiq jobs, please consider doing rename or remove queue in major release only. +Sidekiq jobs, please consider removing the worker in a major release only. + +### Renaming queues + +For the same reasons that removing workers is dangerous, care should be taken +when renaming queues. + +When renaming queues, use the `sidekiq_queue_migrate` helper migration method, +as show in this example: + +```ruby +class MigrateTheRenamedSidekiqQueue < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + sidekiq_queue_migrate 'old_queue_name', to: 'new_queue_name' + end + + def down + sidekiq_queue_migrate 'new_queue_name', to: 'old_queue_name' + end +end + +``` diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 270c6255350..27d6cc9b47b 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -969,7 +969,6 @@ The following table lists variables related to security tools. | **Variable** | **Description** | | `SAST_CONFIDENCE_LEVEL` | Minimum confidence level of security issues you want to be reported; `1` for Low, `2` for Medium, `3` for High. Defaults to `3`. | -| `DS_DISABLE_REMOTE_CHECKS` | Whether remote Dependency Scanning checks are disabled. Defaults to `"false"`. Set to `"true"` to disable checks that send data to GitLab central servers. [Read more about remote checks](../../user/application_security/dependency_scanning/index.md#remote-checks). | #### Disable jobs diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md index 9f87d79025e..15f6ded2587 100644 --- a/doc/user/application_security/dependency_scanning/index.md +++ b/doc/user/application_security/dependency_scanning/index.md @@ -63,23 +63,6 @@ The following languages and dependency managers are supported. | Python ([poetry](https://poetry.eustace.io/)) | not currently ([issue](https://gitlab.com/gitlab-org/gitlab/issues/7006 "Support Poetry in Dependency Scanning")) | not available | | Ruby ([gem](https://rubygems.org/)) | yes | [gemnasium](https://gitlab.com/gitlab-org/security-products/gemnasium), [bundler-audit](https://github.com/rubysec/bundler-audit) | -## Remote checks - -While some tools pull a local database to check vulnerabilities, some others -like Gemnasium require sending data to GitLab central servers to analyze them: - -1. Gemnasium scans the dependencies of your project locally and sends a list of - packages to GitLab central servers. -1. The servers return the list of known vulnerabilities for all versions of - these packages. -1. The client picks up the relevant vulnerabilities by comparing with the versions - of the packages that are used by the project. - -The Gemnasium client does **NOT** send the exact package versions your project relies on. - -You can disable the remote checks by [using](#customizing-the-dependency-scanning-settings) -the `DS_DISABLE_REMOTE_CHECKS` environment variable and setting it to `"true"`. - ## Configuration For GitLab 11.9 and later, to enable Dependency Scanning, you must @@ -116,7 +99,7 @@ include: template: Dependency-Scanning.gitlab-ci.yml variables: - DS_DISABLE_REMOTE_CHECKS: "true" + DS_PYTHON_VERSION: 2 ``` Because template is [evaluated before](../../../ci/yaml/README.md#include) the pipeline @@ -150,7 +133,6 @@ using environment variables. | `DS_PYTHON_VERSION` | Version of Python. If set to 2, dependencies are installed using Python 2.7 instead of Python 3.6. ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12296) in GitLab 12.1)| | | `DS_PIP_DEPENDENCY_PATH` | Path to load Python pip dependencies from. ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12412) in GitLab 12.2) | | | `DS_DEFAULT_ANALYZERS` | Override the names of the official default images. Read more about [customizing analyzers](analyzers.md). | | -| `DS_DISABLE_REMOTE_CHECKS` | Do not send any data to GitLab. Used in the [Gemnasium analyzer](#remote-checks). | | | `DS_PULL_ANALYZER_IMAGES` | Pull the images from the Docker registry (set to `0` to disable). | | | `DS_EXCLUDED_PATHS` | Exclude vulnerabilities from output based on the paths. A comma-separated list of patterns. Patterns can be globs, file or folder paths. Parent directories will also match patterns. | `DS_EXCLUDED_PATHS=doc,spec` | | `DS_DOCKER_CLIENT_NEGOTIATION_TIMEOUT` | Time limit for Docker client negotiation. Timeouts are parsed using Go's [`ParseDuration`](https://golang.org/pkg/time/#ParseDuration). Valid time units are `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`. For example, `300ms`, `1.5h`, or `2h45m`. | | diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 9c00e3c8910..1aafe5804c0 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -148,6 +148,8 @@ excluded_attributes: - :emails_disabled - :max_pages_size - :max_artifacts_size + - :marked_for_deletion_at + - :marked_for_deletion_by_user_id namespaces: - :runners_token - :runners_token_encrypted diff --git a/lib/gitlab/project_authorizations.rb b/lib/gitlab/project_authorizations.rb index a9270cd536e..4e5e2d4a6a9 100644 --- a/lib/gitlab/project_authorizations.rb +++ b/lib/gitlab/project_authorizations.rb @@ -57,7 +57,7 @@ module Gitlab private # Builds a recursive CTE that gets all the groups the current user has - # access to, including any nested groups. + # access to, including any nested groups and any shared groups. def recursive_cte cte = Gitlab::SQL::RecursiveCTE.new(:namespaces_cte) members = Member.arel_table @@ -68,20 +68,27 @@ module Gitlab .select([namespaces[:id], members[:access_level]]) .except(:order) + if Feature.enabled?(:share_group_with_group) + # Namespaces shared with any of the group + cte << Group.select([namespaces[:id], 'group_group_links.group_access AS access_level']) + .joins(join_group_group_links) + .joins(join_members_on_group_group_links) + end + # Sub groups of any groups the user is a member of. cte << Group.select([ namespaces[:id], greatest(members[:access_level], cte.table[:access_level], 'access_level') ]) .joins(join_cte(cte)) - .joins(join_members) + .joins(join_members_on_namespaces) .except(:order) cte end # Builds a LEFT JOIN to join optional memberships onto the CTE. - def join_members + def join_members_on_namespaces members = Member.arel_table namespaces = Namespace.arel_table @@ -94,6 +101,23 @@ module Gitlab Arel::Nodes::OuterJoin.new(members, Arel::Nodes::On.new(cond)) end + def join_group_group_links + group_group_links = GroupGroupLink.arel_table + namespaces = Namespace.arel_table + + cond = group_group_links[:shared_group_id].eq(namespaces[:id]) + Arel::Nodes::InnerJoin.new(group_group_links, Arel::Nodes::On.new(cond)) + end + + def join_members_on_group_group_links + group_group_links = GroupGroupLink.arel_table + members = Member.arel_table + + cond = group_group_links[:shared_with_group_id].eq(members[:source_id]) + .and(members[:user_id].eq(user.id)) + Arel::Nodes::InnerJoin.new(members, Arel::Nodes::On.new(cond)) + end + # Builds an INNER JOIN to join namespaces onto the CTE. def join_cte(cte) namespaces = Namespace.arel_table diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a0b661c023f..3ff79c23071 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5628,9 +5628,6 @@ msgstr "" msgid "Disable" msgstr "" -msgid "Disable email notifications" -msgstr "" - msgid "Disable for this project" msgstr "" @@ -11202,9 +11199,6 @@ msgstr "" msgid "Note: Consider asking your GitLab administrator to configure %{github_integration_link}, which will allow login via GitHub and allow importing repositories without generating a Personal Access Token." msgstr "" -msgid "Note: the container registry is always visible when a project is public" -msgstr "" - msgid "NoteForm|Note" msgstr "" @@ -12869,12 +12863,18 @@ msgstr "" msgid "ProjectSettings|All discussions must be resolved" msgstr "" +msgid "ProjectSettings|Allow users to request access" +msgstr "" + msgid "ProjectSettings|Automatically resolve merge request diff discussions when they become outdated" msgstr "" msgid "ProjectSettings|Badges" msgstr "" +msgid "ProjectSettings|Build, test, and deploy your changes" +msgstr "" + msgid "ProjectSettings|Choose your merge method, merge options, and merge checks." msgstr "" @@ -12884,15 +12884,30 @@ msgstr "" msgid "ProjectSettings|Contact an admin to change this setting." msgstr "" +msgid "ProjectSettings|Container registry" +msgstr "" + msgid "ProjectSettings|Customize your project badges." msgstr "" +msgid "ProjectSettings|Disable email notifications" +msgstr "" + msgid "ProjectSettings|Enable 'Delete source branch' option by default" msgstr "" msgid "ProjectSettings|Every merge creates a merge commit" msgstr "" +msgid "ProjectSettings|Every project can have its own space to store its Docker images" +msgstr "" + +msgid "ProjectSettings|Every project can have its own space to store its packages" +msgstr "" + +msgid "ProjectSettings|Everyone" +msgstr "" + msgid "ProjectSettings|Existing merge requests and protected branches are not affected" msgstr "" @@ -12908,9 +12923,24 @@ msgstr "" msgid "ProjectSettings|Fast-forward merges only" msgstr "" +msgid "ProjectSettings|Git Large File Storage" +msgstr "" + +msgid "ProjectSettings|Internal" +msgstr "" + +msgid "ProjectSettings|Issues" +msgstr "" + msgid "ProjectSettings|Learn more about badges." msgstr "" +msgid "ProjectSettings|Lightweight issue tracking system for this project" +msgstr "" + +msgid "ProjectSettings|Manages large files such as audio, video, and graphics files" +msgstr "" + msgid "ProjectSettings|Merge checks" msgstr "" @@ -12929,24 +12959,60 @@ msgstr "" msgid "ProjectSettings|Merge pipelines will try to validate the post-merge result prior to merging" msgstr "" +msgid "ProjectSettings|Merge requests" +msgstr "" + msgid "ProjectSettings|No merge commits are created" msgstr "" +msgid "ProjectSettings|Note: the container registry is always visible when a project is public" +msgstr "" + msgid "ProjectSettings|Only signed commits can be pushed to this repository." msgstr "" +msgid "ProjectSettings|Packages" +msgstr "" + msgid "ProjectSettings|Pages" msgstr "" +msgid "ProjectSettings|Pages for project documentation" +msgstr "" + +msgid "ProjectSettings|Pipelines" +msgstr "" + msgid "ProjectSettings|Pipelines must succeed" msgstr "" msgid "ProjectSettings|Pipelines need to be configured to enable this feature." msgstr "" +msgid "ProjectSettings|Private" +msgstr "" + +msgid "ProjectSettings|Project visibility" +msgstr "" + +msgid "ProjectSettings|Public" +msgstr "" + +msgid "ProjectSettings|Repository" +msgstr "" + +msgid "ProjectSettings|Share code pastes with others out of Git repository" +msgstr "" + msgid "ProjectSettings|Show link to create/view merge request when pushing from the command line" msgstr "" +msgid "ProjectSettings|Snippets" +msgstr "" + +msgid "ProjectSettings|Submit changes to be merged upstream" +msgstr "" + msgid "ProjectSettings|These checks must pass before merge requests can be merged" msgstr "" @@ -12959,15 +13025,27 @@ msgstr "" msgid "ProjectSettings|This setting will be applied to all projects unless overridden by an admin." msgstr "" +msgid "ProjectSettings|This setting will override user notification preferences for all project members." +msgstr "" + msgid "ProjectSettings|This will dictate the commit history when you merge a merge request" msgstr "" msgid "ProjectSettings|Users can only push commits to this repository that were committed with one of their own verified emails." msgstr "" +msgid "ProjectSettings|View and edit files in this project" +msgstr "" + msgid "ProjectSettings|When conflicts arise the user is given the option to rebase" msgstr "" +msgid "ProjectSettings|Wiki" +msgstr "" + +msgid "ProjectSettings|With GitLab Pages you can host your static websites on GitLab" +msgstr "" + msgid "ProjectTemplates|.NET Core" msgstr "" @@ -16437,6 +16515,9 @@ msgstr "" msgid "The group and its projects can only be viewed by members." msgstr "" +msgid "The group has already been shared with this group" +msgstr "" + msgid "The group settings for %{group_links} require you to enable Two-Factor Authentication for your account. You can %{leave_group_links}." msgstr "" @@ -17031,9 +17112,6 @@ msgstr "" msgid "This setting can be overridden in each project." msgstr "" -msgid "This setting will override user notification preferences for all project members." -msgstr "" - msgid "This setting will update the hostname that is used to generate private commit emails. %{learn_more}" msgstr "" @@ -18929,9 +19007,6 @@ msgstr "" msgid "Will deploy to" msgstr "" -msgid "With GitLab Pages you can host your static websites on GitLab" -msgstr "" - msgid "With contribution analytics you can have an overview for the activity of issues, merge requests and push events of your organization and its members." msgstr "" diff --git a/scripts/review_apps/review-apps.sh b/scripts/review_apps/review-apps.sh index 51768d07860..fbef3ebd231 100755 --- a/scripts/review_apps/review-apps.sh +++ b/scripts/review_apps/review-apps.sh @@ -195,9 +195,22 @@ function download_chart() { helm dependency build . } +function base_config_changed() { + git fetch origin master --depth=50 + + [ -n "$(git diff origin/master... --name-only -- scripts/review_apps/base-config.yaml)" ] +} + function deploy() { local name="$CI_ENVIRONMENT_SLUG" local edition="${GITLAB_EDITION-ce}" + local base_config_file_ref="master" + echo "REVIEW_APP_CONFIG_CHANGED: ${REVIEW_APP_CONFIG_CHANGED}" + if [ -n "${REVIEW_APP_CONFIG_CHANGED}" ]; then + base_config_file_ref="$CI_COMMIT_SHA" + fi + local base_config_file="https://gitlab.com/gitlab-org/gitlab/raw/${base_config_file_ref}/scripts/review_apps/base-config.yaml" + echoinfo "Deploying ${name}..." true IMAGE_REPOSITORY="registry.gitlab.com/gitlab-org/build/cng-mirror" @@ -240,11 +253,11 @@ EOF ) HELM_CMD=$(cat << EOF - $HELM_CMD \ + ${HELM_CMD} \ --namespace="$KUBE_NAMESPACE" \ - --version="$CI_PIPELINE_ID-$CI_JOB_ID" \ - -f "../scripts/review_apps/base-config.yaml" \ - "$name" . + --version="${CI_PIPELINE_ID}-${CI_JOB_ID}" \ + -f "${base_config_file}" \ + "${name}" . EOF ) diff --git a/spec/controllers/groups/group_links_controller_spec.rb b/spec/controllers/groups/group_links_controller_spec.rb new file mode 100644 index 00000000000..8f04822fee6 --- /dev/null +++ b/spec/controllers/groups/group_links_controller_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Groups::GroupLinksController do + let(:shared_with_group) { create(:group, :private) } + let(:shared_group) { create(:group, :private) } + let(:user) { create(:user) } + + before do + sign_in(user) + end + + describe '#create' do + let(:shared_with_group_id) { shared_with_group.id } + + subject do + post(:create, + params: { group_id: shared_group, + shared_with_group_id: shared_with_group_id, + shared_group_access: GroupGroupLink.default_access }) + end + + context 'when user has correct access to both groups' do + let(:group_member) { create(:user) } + + before do + shared_with_group.add_developer(user) + shared_group.add_owner(user) + + shared_with_group.add_developer(group_member) + end + + it 'links group with selected group' do + expect { subject }.to change { shared_with_group.shared_groups.include?(shared_group) }.from(false).to(true) + end + + it 'redirects to group links page' do + subject + + expect(response).to(redirect_to(group_group_members_path(shared_group))) + end + + it 'allows access for group member' do + expect { subject }.to change { group_member.can?(:read_group, shared_group) }.from(false).to(true) + end + + context 'when shared with group id is not present' do + let(:shared_with_group_id) { nil } + + it 'redirects to group links page' do + subject + + expect(response).to(redirect_to(group_group_members_path(shared_group))) + expect(flash[:alert]).to eq('Please select a group.') + end + end + + context 'when link is not persisted in the database' do + before do + allow(::Groups::GroupLinks::CreateService).to( + receive_message_chain(:new, :execute) + .and_return({ status: :error, + http_status: 409, + message: 'error' })) + end + + it 'redirects to group links page' do + subject + + expect(response).to(redirect_to(group_group_members_path(shared_group))) + expect(flash[:alert]).to eq('error') + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(share_group_with_group: false) + end + + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + end + + context 'when user does not have access to the group' do + before do + shared_group.add_owner(user) + end + + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when user does not have admin access to the shared group' do + before do + shared_with_group.add_developer(user) + shared_group.add_developer(user) + end + + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + end +end diff --git a/spec/factories/group_group_links.rb b/spec/factories/group_group_links.rb new file mode 100644 index 00000000000..0711a15b8dd --- /dev/null +++ b/spec/factories/group_group_links.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :group_group_link do + shared_group { create(:group) } + shared_with_group { create(:group) } + group_access { GroupMember::DEVELOPER } + end +end diff --git a/spec/frontend/monitoring/components/charts/anomaly_spec.js b/spec/frontend/monitoring/components/charts/anomaly_spec.js new file mode 100644 index 00000000000..6707d0b1fe8 --- /dev/null +++ b/spec/frontend/monitoring/components/charts/anomaly_spec.js @@ -0,0 +1,303 @@ +import Anomaly from '~/monitoring/components/charts/anomaly.vue'; + +import { shallowMount } from '@vue/test-utils'; +import { colorValues } from '~/monitoring/constants'; +import { + anomalyDeploymentData, + mockProjectDir, + anomalyMockGraphData, + anomalyMockResultValues, +} from '../../mock_data'; +import { TEST_HOST } from 'helpers/test_constants'; +import MonitorTimeSeriesChart from '~/monitoring/components/charts/time_series.vue'; + +const mockWidgets = 'mockWidgets'; +const mockProjectPath = `${TEST_HOST}${mockProjectDir}`; + +jest.mock('~/lib/utils/icon_utils'); // mock getSvgIconPathContent + +const makeAnomalyGraphData = (datasetName, template = anomalyMockGraphData) => { + const queries = anomalyMockResultValues[datasetName].map((values, index) => ({ + ...template.queries[index], + result: [ + { + metrics: {}, + values, + }, + ], + })); + return { ...template, queries }; +}; + +describe('Anomaly chart component', () => { + let wrapper; + + const setupAnomalyChart = props => { + wrapper = shallowMount(Anomaly, { + propsData: { ...props }, + slots: { + default: mockWidgets, + }, + sync: false, + }); + }; + const findTimeSeries = () => wrapper.find(MonitorTimeSeriesChart); + const getTimeSeriesProps = () => findTimeSeries().props(); + + describe('wrapped monitor-time-series-chart component', () => { + const dataSetName = 'noAnomaly'; + const dataSet = anomalyMockResultValues[dataSetName]; + const inputThresholds = ['some threshold']; + + beforeEach(() => { + setupAnomalyChart({ + graphData: makeAnomalyGraphData(dataSetName), + deploymentData: anomalyDeploymentData, + thresholds: inputThresholds, + projectPath: mockProjectPath, + }); + }); + + it('is a Vue instance', () => { + expect(findTimeSeries().exists()).toBe(true); + expect(findTimeSeries().isVueInstance()).toBe(true); + }); + + describe('receives props correctly', () => { + describe('graph-data', () => { + it('receives a single "metric" series', () => { + const { graphData } = getTimeSeriesProps(); + expect(graphData.queries.length).toBe(1); + }); + + it('receives "metric" with all data', () => { + const { graphData } = getTimeSeriesProps(); + const query = graphData.queries[0]; + const expectedQuery = makeAnomalyGraphData(dataSetName).queries[0]; + expect(query).toEqual(expectedQuery); + }); + + it('receives the "metric" results', () => { + const { graphData } = getTimeSeriesProps(); + const { result } = graphData.queries[0]; + const { values } = result[0]; + const [metricDataset] = dataSet; + expect(values).toEqual(expect.any(Array)); + + values.forEach(([, y], index) => { + expect(y).toBeCloseTo(metricDataset[index][1]); + }); + }); + }); + + describe('option', () => { + let option; + let series; + + beforeEach(() => { + ({ option } = getTimeSeriesProps()); + ({ series } = option); + }); + + it('contains a boundary band', () => { + expect(series).toEqual(expect.any(Array)); + expect(series.length).toEqual(2); // 1 upper + 1 lower boundaries + expect(series[0].stack).toEqual(series[1].stack); + + series.forEach(s => { + expect(s.type).toBe('line'); + expect(s.lineStyle.width).toBe(0); + expect(s.lineStyle.color).toMatch(/rgba\(.+\)/); + expect(s.lineStyle.color).toMatch(s.color); + expect(s.symbol).toEqual('none'); + }); + }); + + it('upper boundary values are stacked on top of lower boundary', () => { + const [lowerSeries, upperSeries] = series; + const [, upperDataset, lowerDataset] = dataSet; + + lowerSeries.data.forEach(([, y], i) => { + expect(y).toBeCloseTo(lowerDataset[i][1]); + }); + + upperSeries.data.forEach(([, y], i) => { + expect(y).toBeCloseTo(upperDataset[i][1] - lowerDataset[i][1]); + }); + }); + }); + + describe('series-config', () => { + let seriesConfig; + + beforeEach(() => { + ({ seriesConfig } = getTimeSeriesProps()); + }); + + it('display symbols is enabled', () => { + expect(seriesConfig).toEqual( + expect.objectContaining({ + type: 'line', + symbol: 'circle', + showSymbol: true, + symbolSize: expect.any(Function), + itemStyle: { + color: expect.any(Function), + }, + }), + ); + }); + it('does not display anomalies', () => { + const { symbolSize, itemStyle } = seriesConfig; + const [metricDataset] = dataSet; + + metricDataset.forEach((v, dataIndex) => { + const size = symbolSize(null, { dataIndex }); + const color = itemStyle.color({ dataIndex }); + + // normal color and small size + expect(size).toBeCloseTo(0); + expect(color).toBe(colorValues.primaryColor); + }); + }); + + it('can format y values (to use in tooltips)', () => { + expect(parseFloat(wrapper.vm.yValueFormatted(0, 0))).toEqual(dataSet[0][0][1]); + expect(parseFloat(wrapper.vm.yValueFormatted(1, 0))).toEqual(dataSet[1][0][1]); + expect(parseFloat(wrapper.vm.yValueFormatted(2, 0))).toEqual(dataSet[2][0][1]); + }); + }); + + describe('inherited properties', () => { + it('"deployment-data" keeps the same value', () => { + const { deploymentData } = getTimeSeriesProps(); + expect(deploymentData).toEqual(anomalyDeploymentData); + }); + it('"thresholds" keeps the same value', () => { + const { thresholds } = getTimeSeriesProps(); + expect(thresholds).toEqual(inputThresholds); + }); + it('"projectPath" keeps the same value', () => { + const { projectPath } = getTimeSeriesProps(); + expect(projectPath).toEqual(mockProjectPath); + }); + }); + }); + }); + + describe('with no boundary data', () => { + const dataSetName = 'noBoundary'; + const dataSet = anomalyMockResultValues[dataSetName]; + + beforeEach(() => { + setupAnomalyChart({ + graphData: makeAnomalyGraphData(dataSetName), + deploymentData: anomalyDeploymentData, + }); + }); + + describe('option', () => { + let option; + let series; + + beforeEach(() => { + ({ option } = getTimeSeriesProps()); + ({ series } = option); + }); + + it('does not display a boundary band', () => { + expect(series).toEqual(expect.any(Array)); + expect(series.length).toEqual(0); // no boundaries + }); + + it('can format y values (to use in tooltips)', () => { + expect(parseFloat(wrapper.vm.yValueFormatted(0, 0))).toEqual(dataSet[0][0][1]); + expect(wrapper.vm.yValueFormatted(1, 0)).toBe(''); // missing boundary + expect(wrapper.vm.yValueFormatted(2, 0)).toBe(''); // missing boundary + }); + }); + }); + + describe('with one anomaly', () => { + const dataSetName = 'oneAnomaly'; + const dataSet = anomalyMockResultValues[dataSetName]; + + beforeEach(() => { + setupAnomalyChart({ + graphData: makeAnomalyGraphData(dataSetName), + deploymentData: anomalyDeploymentData, + }); + }); + + describe('series-config', () => { + it('displays one anomaly', () => { + const { seriesConfig } = getTimeSeriesProps(); + const { symbolSize, itemStyle } = seriesConfig; + const [metricDataset] = dataSet; + + const bigDots = metricDataset.filter((v, dataIndex) => { + const size = symbolSize(null, { dataIndex }); + return size > 0.1; + }); + const redDots = metricDataset.filter((v, dataIndex) => { + const color = itemStyle.color({ dataIndex }); + return color === colorValues.anomalySymbol; + }); + + expect(bigDots.length).toBe(1); + expect(redDots.length).toBe(1); + }); + }); + }); + + describe('with offset', () => { + const dataSetName = 'negativeBoundary'; + const dataSet = anomalyMockResultValues[dataSetName]; + const expectedOffset = 4; // Lowst point in mock data is -3.70, it gets rounded + + beforeEach(() => { + setupAnomalyChart({ + graphData: makeAnomalyGraphData(dataSetName), + deploymentData: anomalyDeploymentData, + }); + }); + + describe('receives props correctly', () => { + describe('graph-data', () => { + it('receives a single "metric" series', () => { + const { graphData } = getTimeSeriesProps(); + expect(graphData.queries.length).toBe(1); + }); + + it('receives "metric" results and applies the offset to them', () => { + const { graphData } = getTimeSeriesProps(); + const { result } = graphData.queries[0]; + const { values } = result[0]; + const [metricDataset] = dataSet; + expect(values).toEqual(expect.any(Array)); + + values.forEach(([, y], index) => { + expect(y).toBeCloseTo(metricDataset[index][1] + expectedOffset); + }); + }); + }); + }); + + describe('option', () => { + it('upper boundary values are stacked on top of lower boundary, plus the offset', () => { + const { option } = getTimeSeriesProps(); + const { series } = option; + const [lowerSeries, upperSeries] = series; + const [, upperDataset, lowerDataset] = dataSet; + + lowerSeries.data.forEach(([, y], i) => { + expect(y).toBeCloseTo(lowerDataset[i][1] + expectedOffset); + }); + + upperSeries.data.forEach(([, y], i) => { + expect(y).toBeCloseTo(upperDataset[i][1] - lowerDataset[i][1]); + }); + }); + }); + }); +}); diff --git a/spec/frontend/monitoring/mock_data.js b/spec/frontend/monitoring/mock_data.js new file mode 100644 index 00000000000..39b4343d0ce --- /dev/null +++ b/spec/frontend/monitoring/mock_data.js @@ -0,0 +1,161 @@ +export const mockProjectDir = '/frontend-fixtures/environments-project'; + +export const anomalyDeploymentData = [ + { + id: 111, + iid: 3, + sha: 'f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187', + ref: { + name: 'master', + }, + created_at: '2019-08-19T22:00:00.000Z', + deployed_at: '2019-08-19T22:01:00.000Z', + tag: false, + 'last?': true, + }, + { + id: 110, + iid: 2, + sha: 'f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187', + ref: { + name: 'master', + }, + created_at: '2019-08-19T23:00:00.000Z', + deployed_at: '2019-08-19T23:00:00.000Z', + tag: false, + 'last?': false, + }, +]; + +export const anomalyMockResultValues = { + noAnomaly: [ + [ + ['2019-08-19T19:00:00.000Z', 1.25], + ['2019-08-19T20:00:00.000Z', 1.45], + ['2019-08-19T21:00:00.000Z', 1.55], + ['2019-08-19T22:00:00.000Z', 1.48], + ], + [ + // upper boundary + ['2019-08-19T19:00:00.000Z', 2], + ['2019-08-19T20:00:00.000Z', 2.55], + ['2019-08-19T21:00:00.000Z', 2.65], + ['2019-08-19T22:00:00.000Z', 3.0], + ], + [ + // lower boundary + ['2019-08-19T19:00:00.000Z', 0.45], + ['2019-08-19T20:00:00.000Z', 0.65], + ['2019-08-19T21:00:00.000Z', 0.7], + ['2019-08-19T22:00:00.000Z', 0.8], + ], + ], + noBoundary: [ + [ + ['2019-08-19T19:00:00.000Z', 1.25], + ['2019-08-19T20:00:00.000Z', 1.45], + ['2019-08-19T21:00:00.000Z', 1.55], + ['2019-08-19T22:00:00.000Z', 1.48], + ], + [ + // empty upper boundary + ], + [ + // empty lower boundary + ], + ], + oneAnomaly: [ + [ + ['2019-08-19T19:00:00.000Z', 1.25], + ['2019-08-19T20:00:00.000Z', 3.45], // anomaly + ['2019-08-19T21:00:00.000Z', 1.55], + ], + [ + // upper boundary + ['2019-08-19T19:00:00.000Z', 2], + ['2019-08-19T20:00:00.000Z', 2.55], + ['2019-08-19T21:00:00.000Z', 2.65], + ], + [ + // lower boundary + ['2019-08-19T19:00:00.000Z', 0.45], + ['2019-08-19T20:00:00.000Z', 0.65], + ['2019-08-19T21:00:00.000Z', 0.7], + ], + ], + negativeBoundary: [ + [ + ['2019-08-19T19:00:00.000Z', 1.25], + ['2019-08-19T20:00:00.000Z', 3.45], // anomaly + ['2019-08-19T21:00:00.000Z', 1.55], + ], + [ + // upper boundary + ['2019-08-19T19:00:00.000Z', 2], + ['2019-08-19T20:00:00.000Z', 2.55], + ['2019-08-19T21:00:00.000Z', 2.65], + ], + [ + // lower boundary + ['2019-08-19T19:00:00.000Z', -1.25], + ['2019-08-19T20:00:00.000Z', -2.65], + ['2019-08-19T21:00:00.000Z', -3.7], // lowest point + ], + ], +}; + +export const anomalyMockGraphData = { + title: 'Requests Per Second Mock Data', + type: 'anomaly-chart', + weight: 3, + metrics: [ + // Not used + ], + queries: [ + { + metricId: '90', + id: 'metric', + query_range: 'MOCK_PROMETHEUS_METRIC_QUERY_RANGE', + unit: 'RPS', + label: 'Metrics RPS', + metric_id: 90, + prometheus_endpoint_path: 'MOCK_METRIC_PEP', + result: [ + { + metric: {}, + values: [['2019-08-19T19:00:00.000Z', 0]], + }, + ], + }, + { + metricId: '91', + id: 'upper', + query_range: '...', + unit: 'RPS', + label: 'Upper Limit Metrics RPS', + metric_id: 91, + prometheus_endpoint_path: 'MOCK_UPPER_PEP', + result: [ + { + metric: {}, + values: [['2019-08-19T19:00:00.000Z', 0]], + }, + ], + }, + { + metricId: '92', + id: 'lower', + query_range: '...', + unit: 'RPS', + label: 'Lower Limit Metrics RPS', + metric_id: 92, + prometheus_endpoint_path: 'MOCK_LOWER_PEP', + result: [ + { + metric: {}, + values: [['2019-08-19T19:00:00.000Z', 0]], + }, + ], + }, + ], +}; diff --git a/spec/frontend/repository/components/table/row_spec.js b/spec/frontend/repository/components/table/row_spec.js index e539c560975..565a46bb957 100644 --- a/spec/frontend/repository/components/table/row_spec.js +++ b/spec/frontend/repository/components/table/row_spec.js @@ -104,7 +104,7 @@ describe('Repository table row component', () => { if (pushes) { expect(visitUrl).not.toHaveBeenCalled(); } else { - expect(visitUrl).toHaveBeenCalledWith('https://test.com'); + expect(visitUrl).toHaveBeenCalledWith('https://test.com', undefined); } }); diff --git a/spec/javascripts/monitoring/charts/time_series_spec.js b/spec/javascripts/monitoring/charts/time_series_spec.js index 5c718135b90..31ea9ede9de 100644 --- a/spec/javascripts/monitoring/charts/time_series_spec.js +++ b/spec/javascripts/monitoring/charts/time_series_spec.js @@ -29,7 +29,6 @@ describe('Time series component', () => { shallowMount(TimeSeries, { propsData: { graphData: { ...graphData, type }, - containerWidth: 0, deploymentData: store.state.monitoringDashboard.deploymentData, projectPath, }, @@ -82,7 +81,7 @@ describe('Time series component', () => { seriesName: timeSeriesChart.vm.chartData[0].name, componentSubType: type, value: [mockDate, 5.55555], - seriesIndex: 0, + dataIndex: 0, }, ], value: mockDate, @@ -101,11 +100,15 @@ describe('Time series component', () => { it('formats tooltip content', () => { const name = 'Core Usage'; const value = '5.556'; + const dataIndex = 0; const seriesLabel = timeSeriesChart.find(GlChartSeriesLabel); expect(seriesLabel.vm.color).toBe(''); expect(shallowWrapperContainsSlotText(seriesLabel, 'default', name)).toBe(true); - expect(timeSeriesChart.vm.tooltip.content).toEqual([{ name, value, color: undefined }]); + expect(timeSeriesChart.vm.tooltip.content).toEqual([ + { name, value, dataIndex, color: undefined }, + ]); + expect( shallowWrapperContainsSlotText( timeSeriesChart.find(GlAreaChart), @@ -212,6 +215,39 @@ describe('Time series component', () => { }); describe('chartOptions', () => { + describe('are extended by `option`', () => { + const mockSeriesName = 'Extra series 1'; + const mockOption = { + option1: 'option1', + option2: 'option2', + }; + + it('arbitrary options', () => { + timeSeriesChart.setProps({ + option: mockOption, + }); + + expect(timeSeriesChart.vm.chartOptions).toEqual(jasmine.objectContaining(mockOption)); + }); + + it('additional series', () => { + timeSeriesChart.setProps({ + option: { + series: [ + { + name: mockSeriesName, + }, + ], + }, + }); + + const optionSeries = timeSeriesChart.vm.chartOptions.series; + + expect(optionSeries.length).toEqual(2); + expect(optionSeries[0].name).toEqual(mockSeriesName); + }); + }); + describe('yAxis formatter', () => { let format; diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js index 17e7314e214..6f9a2a34ef5 100644 --- a/spec/javascripts/monitoring/mock_data.js +++ b/spec/javascripts/monitoring/mock_data.js @@ -1,3 +1,7 @@ +import { anomalyMockGraphData as importedAnomalyMockGraphData } from '../../frontend/monitoring/mock_data'; + +export const anomalyMockGraphData = importedAnomalyMockGraphData; + export const mockApiEndpoint = `${gl.TEST_HOST}/monitoring/mock`; export const mockProjectPath = '/frontend-fixtures/environments-project'; @@ -975,7 +979,7 @@ export const graphDataPrometheusQuery = { export const graphDataPrometheusQueryRange = { title: 'Super Chart A1', - type: 'area', + type: 'area-chart', weight: 2, metrics: [ { diff --git a/spec/javascripts/monitoring/panel_type_spec.js b/spec/javascripts/monitoring/panel_type_spec.js index a2366e74d43..e3781117eaf 100644 --- a/spec/javascripts/monitoring/panel_type_spec.js +++ b/spec/javascripts/monitoring/panel_type_spec.js @@ -2,7 +2,9 @@ import { shallowMount } from '@vue/test-utils'; import PanelType from '~/monitoring/components/panel_type.vue'; import EmptyChart from '~/monitoring/components/charts/empty_chart.vue'; import TimeSeriesChart from '~/monitoring/components/charts/time_series.vue'; +import AnomalyChart from '~/monitoring/components/charts/anomaly.vue'; import { graphDataPrometheusQueryRange } from './mock_data'; +import { anomalyMockGraphData } from '../../frontend/monitoring/mock_data'; import { createStore } from '~/monitoring/stores'; describe('Panel Type component', () => { @@ -49,17 +51,20 @@ describe('Panel Type component', () => { describe('when Graph data is available', () => { const exampleText = 'example_text'; + const propsData = { + clipboardText: exampleText, + dashboardWidth, + graphData: graphDataPrometheusQueryRange, + }; - beforeEach(() => { + beforeEach(done => { store = createStore(); panelType = shallowMount(PanelType, { - propsData: { - clipboardText: exampleText, - dashboardWidth, - graphData: graphDataPrometheusQueryRange, - }, + propsData, + sync: false, store, }); + panelType.vm.$nextTick(done); }); describe('Time Series Chart panel type', () => { @@ -75,5 +80,19 @@ describe('Panel Type component', () => { expect(clipboardText()).toBe(exampleText); }); }); + + describe('Anomaly Chart panel type', () => { + beforeEach(done => { + panelType.setProps({ + graphData: anomalyMockGraphData, + }); + panelType.vm.$nextTick(done); + }); + + it('is rendered with an anomaly chart', () => { + expect(panelType.find(AnomalyChart).isVueInstance()).toBe(true); + expect(panelType.find(AnomalyChart).exists()).toBe(true); + }); + }); }); }); diff --git a/spec/javascripts/monitoring/utils_spec.js b/spec/javascripts/monitoring/utils_spec.js index 512dd2a0eb3..202b4ec8f2e 100644 --- a/spec/javascripts/monitoring/utils_spec.js +++ b/spec/javascripts/monitoring/utils_spec.js @@ -7,9 +7,14 @@ import { stringToISODate, ISODateToString, isValidDate, + graphDataValidatorForAnomalyValues, } from '~/monitoring/utils'; import { timeWindows, timeWindowsKeyNames } from '~/monitoring/constants'; -import { graphDataPrometheusQuery, graphDataPrometheusQueryRange } from './mock_data'; +import { + graphDataPrometheusQuery, + graphDataPrometheusQueryRange, + anomalyMockGraphData, +} from './mock_data'; describe('getTimeDiff', () => { function secondsBetween({ start, end }) { @@ -307,3 +312,34 @@ describe('isDateTimePickerInputValid', () => { }); }); }); + +describe('graphDataValidatorForAnomalyValues', () => { + let oneQuery; + let threeQueries; + let fourQueries; + beforeEach(() => { + oneQuery = graphDataPrometheusQuery; + threeQueries = anomalyMockGraphData; + + const queries = [...threeQueries.queries]; + queries.push(threeQueries.queries[0]); + fourQueries = { + ...anomalyMockGraphData, + queries, + }; + }); + /* + * Anomaly charts can accept results for exactly 3 queries, + */ + it('validates passes with the right query format', () => { + expect(graphDataValidatorForAnomalyValues(threeQueries)).toBe(true); + }); + + it('validation fails for wrong format, 1 metric', () => { + expect(graphDataValidatorForAnomalyValues(oneQuery)).toBe(false); + }); + + it('validation fails for wrong format, more than 3 metrics', () => { + expect(graphDataValidatorForAnomalyValues(fourQueries)).toBe(false); + }); +}); diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 3b665d95004..d0e4b92410d 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -423,6 +423,7 @@ project: - alerts_service - grafana_integration - remove_source_branch_after_merge +- deleting_user award_emoji: - awardable - user diff --git a/spec/lib/gitlab/project_authorizations_spec.rb b/spec/lib/gitlab/project_authorizations_spec.rb index 82ccb42f8a6..006daa29ea1 100644 --- a/spec/lib/gitlab/project_authorizations_spec.rb +++ b/spec/lib/gitlab/project_authorizations_spec.rb @@ -3,48 +3,55 @@ require 'spec_helper' describe Gitlab::ProjectAuthorizations do - let(:group) { create(:group) } - let!(:owned_project) { create(:project) } - let!(:other_project) { create(:project) } - let!(:group_project) { create(:project, namespace: group) } - - let(:user) { owned_project.namespace.owner } - def map_access_levels(rows) rows.each_with_object({}) do |row, hash| hash[row.project_id] = row.access_level end end - before do - other_project.add_reporter(user) - group.add_developer(user) - end - - let(:authorizations) do + subject(:authorizations) do described_class.new(user).calculate end - it 'returns the correct number of authorizations' do - expect(authorizations.length).to eq(3) - end + context 'user added to group and project' do + let(:group) { create(:group) } + let!(:other_project) { create(:project) } + let!(:group_project) { create(:project, namespace: group) } + let!(:owned_project) { create(:project) } + let(:user) { owned_project.namespace.owner } - it 'includes the correct projects' do - expect(authorizations.pluck(:project_id)) - .to include(owned_project.id, other_project.id, group_project.id) - end + before do + other_project.add_reporter(user) + group.add_developer(user) + end + + it 'returns the correct number of authorizations' do + expect(authorizations.length).to eq(3) + end - it 'includes the correct access levels' do - mapping = map_access_levels(authorizations) + it 'includes the correct projects' do + expect(authorizations.pluck(:project_id)) + .to include(owned_project.id, other_project.id, group_project.id) + end + + it 'includes the correct access levels' do + mapping = map_access_levels(authorizations) - expect(mapping[owned_project.id]).to eq(Gitlab::Access::MAINTAINER) - expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER) - expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER) + expect(mapping[owned_project.id]).to eq(Gitlab::Access::MAINTAINER) + expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER) + expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER) + end end context 'with nested groups' do + let(:group) { create(:group) } let!(:nested_group) { create(:group, parent: group) } let!(:nested_project) { create(:project, namespace: nested_group) } + let(:user) { create(:user) } + + before do + group.add_developer(user) + end it 'includes nested groups' do expect(authorizations.pluck(:project_id)).to include(nested_project.id) @@ -64,4 +71,114 @@ describe Gitlab::ProjectAuthorizations do expect(mapping[nested_project.id]).to eq(Gitlab::Access::MAINTAINER) end end + + context 'with shared groups' do + let(:parent_group_user) { create(:user) } + let(:group_user) { create(:user) } + let(:child_group_user) { create(:user) } + + set(:group_parent) { create(:group, :private) } + set(:group) { create(:group, :private, parent: group_parent) } + set(:group_child) { create(:group, :private, parent: group) } + + set(:shared_group_parent) { create(:group, :private) } + set(:shared_group) { create(:group, :private, parent: shared_group_parent) } + set(:shared_group_child) { create(:group, :private, parent: shared_group) } + + set(:project_parent) { create(:project, group: shared_group_parent) } + set(:project) { create(:project, group: shared_group) } + set(:project_child) { create(:project, group: shared_group_child) } + + before do + group_parent.add_owner(parent_group_user) + group.add_owner(group_user) + group_child.add_owner(child_group_user) + + create(:group_group_link, shared_group: shared_group, shared_with_group: group) + end + + context 'when feature flag share_group_with_group is enabled' do + before do + stub_feature_flags(share_group_with_group: true) + end + + context 'group user' do + let(:user) { group_user } + + it 'creates proper authorizations' do + mapping = map_access_levels(authorizations) + + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to eq(Gitlab::Access::DEVELOPER) + expect(mapping[project_child.id]).to eq(Gitlab::Access::DEVELOPER) + end + end + + context 'parent group user' do + let(:user) { parent_group_user } + + it 'creates proper authorizations' do + mapping = map_access_levels(authorizations) + + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to be_nil + expect(mapping[project_child.id]).to be_nil + end + end + + context 'child group user' do + let(:user) { child_group_user } + + it 'creates proper authorizations' do + mapping = map_access_levels(authorizations) + + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to be_nil + expect(mapping[project_child.id]).to be_nil + end + end + end + + context 'when feature flag share_group_with_group is disabled' do + before do + stub_feature_flags(share_group_with_group: false) + end + + context 'group user' do + let(:user) { group_user } + + it 'creates proper authorizations' do + mapping = map_access_levels(authorizations) + + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to be_nil + expect(mapping[project_child.id]).to be_nil + end + end + + context 'parent group user' do + let(:user) { parent_group_user } + + it 'creates proper authorizations' do + mapping = map_access_levels(authorizations) + + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to be_nil + expect(mapping[project_child.id]).to be_nil + end + end + + context 'child group user' do + let(:user) { child_group_user } + + it 'creates proper authorizations' do + mapping = map_access_levels(authorizations) + + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to be_nil + expect(mapping[project_child.id]).to be_nil + end + end + end + end end diff --git a/spec/models/group_group_link_spec.rb b/spec/models/group_group_link_spec.rb new file mode 100644 index 00000000000..e4ad5703a10 --- /dev/null +++ b/spec/models/group_group_link_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GroupGroupLink do + let_it_be(:group) { create(:group) } + let_it_be(:shared_group) { create(:group) } + let_it_be(:group_group_link) do + create(:group_group_link, shared_group: shared_group, + shared_with_group: group) + end + + describe 'relations' do + it { is_expected.to belong_to(:shared_group) } + it { is_expected.to belong_to(:shared_with_group) } + end + + describe 'validation' do + it { is_expected.to validate_presence_of(:shared_group) } + + it do + is_expected.to( + validate_uniqueness_of(:shared_group_id) + .scoped_to(:shared_with_group_id) + .with_message('The group has already been shared with this group')) + end + + it { is_expected.to validate_presence_of(:shared_with_group) } + it { is_expected.to validate_presence_of(:group_access) } + + it do + is_expected.to( + validate_inclusion_of(:group_access).in_array(Gitlab::Access.values)) + end + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 520421ac5e3..9a89ffb1b2d 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -525,6 +525,128 @@ describe Group do it { expect(subject.parent).to be_kind_of(described_class) } end + describe '#max_member_access_for_user' do + context 'group shared with another group' do + let(:parent_group_user) { create(:user) } + let(:group_user) { create(:user) } + let(:child_group_user) { create(:user) } + + set(:group_parent) { create(:group, :private) } + set(:group) { create(:group, :private, parent: group_parent) } + set(:group_child) { create(:group, :private, parent: group) } + + set(:shared_group_parent) { create(:group, :private) } + set(:shared_group) { create(:group, :private, parent: shared_group_parent) } + set(:shared_group_child) { create(:group, :private, parent: shared_group) } + + before do + group_parent.add_owner(parent_group_user) + group.add_owner(group_user) + group_child.add_owner(child_group_user) + + create(:group_group_link, { shared_with_group: group, + shared_group: shared_group, + group_access: GroupMember::DEVELOPER }) + end + + context 'when feature flag share_group_with_group is enabled' do + before do + stub_feature_flags(share_group_with_group: true) + end + + context 'with user in the group' do + let(:user) { group_user } + + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER) + expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER) + end + end + + context 'with user in the parent group' do + let(:user) { parent_group_user } + + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + end + end + + context 'with user in the child group' do + let(:user) { child_group_user } + + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + end + end + end + + context 'when feature flag share_group_with_group is disabled' do + before do + stub_feature_flags(share_group_with_group: false) + end + + context 'with user in the group' do + let(:user) { group_user } + + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + end + end + + context 'with user in the parent group' do + let(:user) { parent_group_user } + + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + end + end + + context 'with user in the child group' do + let(:user) { child_group_user } + + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + end + end + end + end + + context 'multiple groups shared with group' do + let(:user) { create(:user) } + let(:group) { create(:group, :private) } + let(:shared_group_parent) { create(:group, :private) } + let(:shared_group) { create(:group, :private, parent: shared_group_parent) } + + before do + stub_feature_flags(share_group_with_group: true) + + group.add_owner(user) + + create(:group_group_link, { shared_with_group: group, + shared_group: shared_group, + group_access: GroupMember::DEVELOPER }) + create(:group_group_link, { shared_with_group: group, + shared_group: shared_group_parent, + group_access: GroupMember::MAINTAINER }) + end + + it 'returns correct access level' do + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::MAINTAINER) + end + end + end + describe '#members_with_parents' do let!(:group) { create(:group, :nested) } let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 5c9e5746683..f2942020e16 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -151,9 +151,15 @@ describe API::Members do expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.map { |u| u['id'] }).to eq [developer.id, maintainer.id, nested_user.id, project_user.id, linked_group_user.id] - expect(json_response.map { |u| u['access_level'] }).to eq [Gitlab::Access::DEVELOPER, Gitlab::Access::OWNER, Gitlab::Access::DEVELOPER, - Gitlab::Access::DEVELOPER, Gitlab::Access::DEVELOPER] + + expected_users_and_access_levels = [ + [developer.id, Gitlab::Access::DEVELOPER], + [maintainer.id, Gitlab::Access::OWNER], + [nested_user.id, Gitlab::Access::DEVELOPER], + [project_user.id, Gitlab::Access::DEVELOPER], + [linked_group_user.id, Gitlab::Access::DEVELOPER] + ] + expect(json_response.map { |u| [u['id'], u['access_level']] }).to match_array(expected_users_and_access_levels) end it 'finds all group members including inherited members' do diff --git a/spec/services/groups/group_links/create_service_spec.rb b/spec/services/groups/group_links/create_service_spec.rb new file mode 100644 index 00000000000..ca005536e0d --- /dev/null +++ b/spec/services/groups/group_links/create_service_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Groups::GroupLinks::CreateService, '#execute' do + let(:parent_group_user) { create(:user) } + let(:group_user) { create(:user) } + let(:child_group_user) { create(:user) } + + set(:group_parent) { create(:group, :private) } + set(:group) { create(:group, :private, parent: group_parent) } + set(:group_child) { create(:group, :private, parent: group) } + + set(:shared_group_parent) { create(:group, :private) } + set(:shared_group) { create(:group, :private, parent: shared_group_parent) } + set(:shared_group_child) { create(:group, :private, parent: shared_group) } + + set(:project_parent) { create(:project, group: shared_group_parent) } + set(:project) { create(:project, group: shared_group) } + set(:project_child) { create(:project, group: shared_group_child) } + + let(:opts) do + { + shared_group_access: Gitlab::Access::DEVELOPER, + expires_at: nil + } + end + let(:user) { group_user } + + subject { described_class.new(group, user, opts) } + + before do + group.add_guest(group_user) + shared_group.add_owner(group_user) + end + + it 'adds group to another group' do + expect { subject.execute(shared_group) }.to change { group.shared_group_links.count }.from(0).to(1) + end + + it 'returns false if shared group is blank' do + expect { subject.execute(nil) }.not_to change { group.shared_group_links.count } + end + + context 'user does not have access to group' do + let(:user) { create(:user) } + + before do + shared_group.add_owner(user) + end + + it 'returns error' do + result = subject.execute(shared_group) + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(404) + end + end + + context 'user does not have admin access to shared group' do + let(:user) { create(:user) } + + before do + group.add_guest(user) + shared_group.add_developer(user) + end + + it 'returns error' do + result = subject.execute(shared_group) + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(404) + end + end + + context 'group hierarchies' do + before do + group_parent.add_owner(parent_group_user) + group.add_owner(group_user) + group_child.add_owner(child_group_user) + end + + context 'group user' do + let(:user) { group_user } + + it 'create proper authorizations' do + subject.execute(shared_group) + + expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey + expect(Ability.allowed?(user, :read_project, project)).to be_truthy + expect(Ability.allowed?(user, :read_project, project_child)).to be_truthy + end + end + + context 'parent group user' do + let(:user) { parent_group_user } + + it 'create proper authorizations' do + subject.execute(shared_group) + + expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey + expect(Ability.allowed?(user, :read_project, project)).to be_falsey + expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey + end + end + + context 'child group user' do + let(:user) { child_group_user } + + it 'create proper authorizations' do + subject.execute(shared_group) + + expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey + expect(Ability.allowed?(user, :read_project, project)).to be_falsey + expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey + end + end + end +end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index b7ba4d61723..5ceb54eb2d5 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -21,8 +21,8 @@ describe 'Every Sidekiq worker' do missing_from_file = worker_queues - file_worker_queues expect(missing_from_file).to be_empty, "expected #{missing_from_file.to_a.inspect} to be in Gitlab::SidekiqConfig::QUEUE_CONFIG_PATHS" - unncessarily_in_file = file_worker_queues - worker_queues - expect(unncessarily_in_file).to be_empty, "expected #{unncessarily_in_file.to_a.inspect} not to be in Gitlab::SidekiqConfig::QUEUE_CONFIG_PATHS" + unnecessarily_in_file = file_worker_queues - worker_queues + expect(unnecessarily_in_file).to be_empty, "expected #{unnecessarily_in_file.to_a.inspect} not to be in Gitlab::SidekiqConfig::QUEUE_CONFIG_PATHS" end it 'has its queue or namespace in config/sidekiq_queues.yml', :aggregate_failures do @@ -42,7 +42,7 @@ describe 'Every Sidekiq worker' do end # All Sidekiq worker classes should declare a valid `feature_category` - # or explicitely be excluded with the `feature_category_not_owned!` annotation. + # or explicitly be excluded with the `feature_category_not_owned!` annotation. # Please see doc/development/sidekiq_style_guide.md#Feature-Categorization for more details. it 'has a feature_category or feature_category_not_owned! attribute', :aggregate_failures do Gitlab::SidekiqConfig.workers.each do |worker| @@ -62,5 +62,36 @@ describe 'Every Sidekiq worker' do expect(feature_categories).to include(worker.get_feature_category), "expected #{worker.inspect} to declare a valid feature_category, but got #{worker.get_feature_category}" end end + + # Memory-bound workers are very expensive to run, since they need to run on nodes with very low + # concurrency, so that each job can consume a large amounts of memory. For this reason, on + # GitLab.com, when a large number of memory-bound jobs arrive at once, we let them queue up + # rather than scaling the hardware to meet the SLO. For this reason, memory-bound, + # latency-sensitive jobs are explicitly discouraged and disabled. + it 'is (exclusively) memory-bound or latency-sentitive, not both', :aggregate_failures do + latency_sensitive_workers = Gitlab::SidekiqConfig.workers + .select(&:latency_sensitive_worker?) + + latency_sensitive_workers.each do |worker| + expect(worker.get_worker_resource_boundary).not_to eq(:memory), "#{worker.inspect} cannot be both memory-bound and latency sensitive" + end + end + + # In high traffic installations, such as GitLab.com, `latency_sensitive` workers run in a + # dedicated fleet. In order to ensure short queue times, `latency_sensitive` jobs have strict + # SLOs in order to ensure throughput. However, when a worker depends on an external service, + # such as a user's k8s cluster or a third-party internet service, we cannot guarantee latency, + # and therefore throughput. An outage to an 3rd party service could therefore impact throughput + # on other latency_sensitive jobs, leading to degradation through the GitLab application. + # Please see doc/development/sidekiq_style_guide.md#Jobs-with-External-Dependencies for more + # details. + it 'has (exclusively) external dependencies or is latency-sentitive, not both', :aggregate_failures do + latency_sensitive_workers = Gitlab::SidekiqConfig.workers + .select(&:latency_sensitive_worker?) + + latency_sensitive_workers.each do |worker| + expect(worker.worker_has_external_dependencies?).to be_falsey, "#{worker.inspect} cannot have both external dependencies and be latency sensitive" + end + end end end |