diff options
58 files changed, 719 insertions, 122 deletions
diff --git a/.eslintrc.yml b/.eslintrc.yml index 59eec634e8b..524ce64118f 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -1,5 +1,6 @@ extends: - '@gitlab' + - plugin:promise/recommended globals: __webpack_public_path__: true gl: false @@ -42,6 +43,11 @@ rules: no-jquery/no-load: error no-jquery/no-load-shorthand: error no-jquery/no-serialize: error + promise/always-return: off + promise/no-callback-in-promise: off + promise/no-nesting: off + promise/param-names: off + promise/valid-params: off overrides: files: - '**/spec/**/*' diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml index 70f3620aa79..49dd778f4fb 100644 --- a/.gitlab/ci/global.gitlab-ci.yml +++ b/.gitlab/ci/global.gitlab-ci.yml @@ -100,6 +100,7 @@ refs: - master - /^\d+-\d+-auto-deploy-\d+$/ + - /^[\d-]+-stable(-ee)?$/ .only-review-schedules: only: diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 9251644911f..01e4b1afdb0 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -1,8 +1,15 @@ +.except-deploys: + except: + refs: + - /^[\d-]+-stable(-ee)?$/ + - /^\d+-\d+-auto-deploy-\d+$/ + .review-docker: extends: - .default-tags - .default-retry - .default-only + - .except-deploys image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-qa-alpine services: - docker:19.03.0-dind @@ -36,6 +43,7 @@ schedule:review-cleanup: - .default-only - .only-code-qa-changes - .only-review-schedules + - .except-deploys stage: prepare image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-charts-build-base allow_failure: true @@ -52,6 +60,7 @@ schedule:review-cleanup: extends: - .default-only - .only-code-qa-changes + - .except-deploys image: ruby:2.6-alpine stage: review-prepare before_script: @@ -80,6 +89,7 @@ schedule:review-build-cng: - .default-retry - .default-only - .only-code-qa-changes + - .except-deploys stage: review image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-charts-build-base dependencies: [] @@ -257,6 +267,7 @@ parallel-spec-reports: - .default-only - .only-code-qa-changes - .only-review + - .except-deploys image: ruby:2.6-alpine stage: post-test dependencies: ["review-qa-all"] diff --git a/app/assets/javascripts/commons/vue.js b/app/assets/javascripts/commons/vue.js index 8b62d78c043..5b5a1507d38 100644 --- a/app/assets/javascripts/commons/vue.js +++ b/app/assets/javascripts/commons/vue.js @@ -1,5 +1,8 @@ import Vue from 'vue'; +import GlFeatureFlagsPlugin from '~/vue_shared/gl_feature_flags_plugin'; if (process.env.NODE_ENV !== 'production') { Vue.config.productionTip = false; } + +Vue.use(GlFeatureFlagsPlugin); diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index 12a4c83e053..b5ed54a6e25 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -175,7 +175,6 @@ export default { 'metricsWithData', 'useDashboardEndpoint', 'allDashboards', - 'multipleDashboardsEnabled', 'additionalPanelTypesEnabled', ]), firstDashboard() { @@ -318,7 +317,6 @@ export default { <div class="row"> <template v-if="environmentsEndpoint"> <gl-form-group - v-if="multipleDashboardsEnabled" :label="__('Dashboard')" label-size="sm" label-for="monitor-dashboards-dropdown" diff --git a/app/assets/javascripts/monitoring/monitoring_bundle.js b/app/assets/javascripts/monitoring/monitoring_bundle.js index 51cef20455c..6aa1fb5e9c6 100644 --- a/app/assets/javascripts/monitoring/monitoring_bundle.js +++ b/app/assets/javascripts/monitoring/monitoring_bundle.js @@ -14,7 +14,6 @@ export default (props = {}) => { if (gon.features) { store.dispatch('monitoringDashboard/setFeatureFlags', { prometheusEndpointEnabled: gon.features.environmentMetricsUsePrometheusEndpoint, - multipleDashboardsEnabled: gon.features.environmentMetricsShowMultipleDashboards, additionalPanelTypesEnabled: gon.features.environmentMetricsAdditionalPanelTypes, }); } diff --git a/app/assets/javascripts/monitoring/stores/actions.js b/app/assets/javascripts/monitoring/stores/actions.js index 0cbad179f17..2cf34ddb45b 100644 --- a/app/assets/javascripts/monitoring/stores/actions.js +++ b/app/assets/javascripts/monitoring/stores/actions.js @@ -37,10 +37,9 @@ export const setEndpoints = ({ commit }, endpoints) => { export const setFeatureFlags = ( { commit }, - { prometheusEndpointEnabled, multipleDashboardsEnabled, additionalPanelTypesEnabled }, + { prometheusEndpointEnabled, additionalPanelTypesEnabled }, ) => { commit(types.SET_DASHBOARD_ENABLED, prometheusEndpointEnabled); - commit(types.SET_MULTIPLE_DASHBOARDS_ENABLED, multipleDashboardsEnabled); commit(types.SET_ADDITIONAL_PANEL_TYPES_ENABLED, additionalPanelTypesEnabled); }; @@ -51,13 +50,8 @@ export const setShowErrorBanner = ({ commit }, enabled) => { export const requestMetricsDashboard = ({ commit }) => { commit(types.REQUEST_METRICS_DATA); }; -export const receiveMetricsDashboardSuccess = ( - { state, commit, dispatch }, - { response, params }, -) => { - if (state.multipleDashboardsEnabled) { - commit(types.SET_ALL_DASHBOARDS, response.all_dashboards); - } +export const receiveMetricsDashboardSuccess = ({ commit, dispatch }, { response, params }) => { + commit(types.SET_ALL_DASHBOARDS, response.all_dashboards); commit(types.RECEIVE_METRICS_DATA_SUCCESS, response.dashboard.panel_groups); dispatch('fetchPrometheusMetrics', params); }; diff --git a/app/assets/javascripts/monitoring/stores/mutation_types.js b/app/assets/javascripts/monitoring/stores/mutation_types.js index 4b1aadbcf05..9c546427c6e 100644 --- a/app/assets/javascripts/monitoring/stores/mutation_types.js +++ b/app/assets/javascripts/monitoring/stores/mutation_types.js @@ -10,7 +10,6 @@ export const RECEIVE_ENVIRONMENTS_DATA_FAILURE = 'RECEIVE_ENVIRONMENTS_DATA_FAIL export const SET_QUERY_RESULT = 'SET_QUERY_RESULT'; export const SET_TIME_WINDOW = 'SET_TIME_WINDOW'; export const SET_DASHBOARD_ENABLED = 'SET_DASHBOARD_ENABLED'; -export const SET_MULTIPLE_DASHBOARDS_ENABLED = 'SET_MULTIPLE_DASHBOARDS_ENABLED'; export const SET_ADDITIONAL_PANEL_TYPES_ENABLED = 'SET_ADDITIONAL_PANEL_TYPES_ENABLED'; export const SET_ALL_DASHBOARDS = 'SET_ALL_DASHBOARDS'; export const SET_ENDPOINTS = 'SET_ENDPOINTS'; diff --git a/app/assets/javascripts/monitoring/stores/mutations.js b/app/assets/javascripts/monitoring/stores/mutations.js index b1080b23815..320b33d3d69 100644 --- a/app/assets/javascripts/monitoring/stores/mutations.js +++ b/app/assets/javascripts/monitoring/stores/mutations.js @@ -89,9 +89,6 @@ export default { [types.SET_DASHBOARD_ENABLED](state, enabled) { state.useDashboardEndpoint = enabled; }, - [types.SET_MULTIPLE_DASHBOARDS_ENABLED](state, enabled) { - state.multipleDashboardsEnabled = enabled; - }, [types.SET_GETTING_STARTED_EMPTY_STATE](state) { state.emptyState = 'gettingStarted'; }, diff --git a/app/assets/javascripts/monitoring/stores/state.js b/app/assets/javascripts/monitoring/stores/state.js index 440bdc951e0..e894e988f6a 100644 --- a/app/assets/javascripts/monitoring/stores/state.js +++ b/app/assets/javascripts/monitoring/stores/state.js @@ -8,7 +8,6 @@ export default () => ({ deploymentsEndpoint: null, dashboardEndpoint: invalidUrl, useDashboardEndpoint: false, - multipleDashboardsEnabled: false, additionalPanelTypesEnabled: false, emptyState: 'gettingStarted', showEmptyState: true, diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 666b3f025f5..da1a7c290f8 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -7,6 +7,7 @@ import _ from 'underscore'; import axios from './lib/utils/axios_utils'; import { s__, __, sprintf } from './locale'; import ModalStore from './boards/stores/modal_store'; +import { parseBoolean } from './lib/utils/common_utils'; // TODO: remove eventHub hack after code splitting refactor window.emitSidebarEvent = window.emitSidebarEvent || $.noop; @@ -275,12 +276,13 @@ function UsersSelect(currentUser, els, options = {}) { }) .map(input => { const userId = parseInt(input.value, 10); - const { avatarUrl, avatar_url, name, username } = input.dataset; + const { avatarUrl, avatar_url, name, username, canMerge } = input.dataset; return { avatar_url: avatarUrl || avatar_url, id: userId, name, username, + can_merge: parseBoolean(canMerge), }; }); diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue index 91c0b40a0b5..8132b1a944b 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue @@ -1,5 +1,7 @@ <script> import $ from 'jquery'; +import { __ } from '~/locale'; +import createFlash from '~/flash'; import statusIcon from '../mr_widget_status_icon.vue'; import tooltip from '../../../vue_shared/directives/tooltip'; import eventHub from '../../event_hub'; @@ -29,12 +31,12 @@ export default { .then(res => res.data) .then(data => { eventHub.$emit('UpdateWidgetData', data); - new window.Flash('The merge request can now be merged.', 'notice'); // eslint-disable-line + createFlash(__('The merge request can now be merged.'), 'notice'); $('.merge-request .detail-page-description .title').text(this.mr.title); }) .catch(() => { this.isMakingRequest = false; - new window.Flash('Something went wrong. Please try again.'); // eslint-disable-line + createFlash(__('Something went wrong. Please try again.')); }); }, }, diff --git a/app/assets/javascripts/vue_shared/gl_feature_flags_plugin.js b/app/assets/javascripts/vue_shared/gl_feature_flags_plugin.js new file mode 100644 index 00000000000..3488a44bd0f --- /dev/null +++ b/app/assets/javascripts/vue_shared/gl_feature_flags_plugin.js @@ -0,0 +1,7 @@ +export default Vue => { + Vue.mixin({ + provide: { + glFeatures: { ...((window.gon && window.gon.features) || {}) }, + }, + }); +}; diff --git a/app/assets/javascripts/vue_shared/mixins/gl_feature_flags_mixin.js b/app/assets/javascripts/vue_shared/mixins/gl_feature_flags_mixin.js new file mode 100644 index 00000000000..dc8a63f26ac --- /dev/null +++ b/app/assets/javascripts/vue_shared/mixins/gl_feature_flags_mixin.js @@ -0,0 +1,8 @@ +export default () => ({ + inject: { + glFeatures: { + from: 'glFeatures', + default: () => ({}), + }, + }, +}); diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index d67a0f83aa2..f4928627748 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -245,14 +245,6 @@ $note-form-margin-left: 72px; } } - .note-header { - @include notes-media('max', map-get($grid-breakpoints, xs)) { - .inline { - display: block; - } - } - } - .note-emoji-button { position: relative; line-height: 1; @@ -635,10 +627,6 @@ $note-form-margin-left: 72px; .note-headline-light { display: inline; - - @include notes-media('max', map-get($grid-breakpoints, xs)) { - display: block; - } } .note-headline-light, diff --git a/app/controllers/concerns/renders_assignees.rb b/app/controllers/concerns/renders_assignees.rb new file mode 100644 index 00000000000..e9583a7a530 --- /dev/null +++ b/app/controllers/concerns/renders_assignees.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module RendersAssignees + def preload_assignees_for_render(merge_request) + merge_request.project.team.max_member_access_for_user_ids(merge_request.assignees.map(&:id)) + end +end diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 64de0e665d3..841fc343b30 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -12,7 +12,6 @@ class Projects::EnvironmentsController < Projects::ApplicationController before_action :expire_etag_cache, only: [:index] before_action only: [:metrics, :additional_metrics, :metrics_dashboard] do push_frontend_feature_flag(:environment_metrics_use_prometheus_endpoint, default_enabled: true) - push_frontend_feature_flag(:environment_metrics_show_multiple_dashboards) push_frontend_feature_flag(:environment_metrics_additional_panel_types) push_frontend_feature_flag(:prometheus_computed_alerts) end @@ -168,7 +167,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController dashboard_path: params[:dashboard], **dashboard_params.to_h.symbolize_keys ) - elsif Feature.enabled?(:environment_metrics_show_multiple_dashboards, project) + else result = dashboard_finder.find( project, current_user, @@ -177,8 +176,6 @@ class Projects::EnvironmentsController < Projects::ApplicationController ) result[:all_dashboards] = dashboard_finder.find_all_paths(project) - else - result = dashboard_finder.find(project, current_user, environment: environment) end respond_to do |format| diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index e51ce752233..2f73fccabcf 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -5,6 +5,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo include IssuableActions include RendersNotes include RendersCommits + include RendersAssignees include ToggleAwardEmoji include IssuableCollections include RecordUserLastActivity @@ -41,6 +42,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo # use next to appease Rubocop next render('invalid') if target_branch_missing? + preload_assignees_for_render(@merge_request) + # Build a note object for comment form @note = @project.notes.new(noteable: @merge_request) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 24614b5030c..df9d1933271 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -372,6 +372,12 @@ module IssuablesHelper finder.class.scalar_params.any? { |p| params[p].present? } end + def assignee_sidebar_data(assignee, merge_request: nil) + { avatar_url: assignee.avatar_url, name: assignee.name, username: assignee.username }.tap do |data| + data[:can_merge] = merge_request.can_be_merged_by?(assignee) if merge_request + end + end + private def sidebar_gutter_collapsed? diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 8fad42436ca..2b913808c2f 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -40,7 +40,7 @@ .info-well .well-segment.admin-well.admin-well-features %h4 Features - = feature_entry(_('Sign up'), href: admin_application_settings_path(anchor: 'js-signup-settings')) + = feature_entry(_('Sign up'), href: admin_application_settings_path(anchor: 'js-signup-settings'), enabled: allow_signup?) = feature_entry(_('LDAP'), enabled: Gitlab.config.ldap.enabled) = feature_entry(_('Gravatar'), href: admin_application_settings_path(anchor: 'js-account-settings'), enabled: gravatar_enabled?) = feature_entry(_('OmniAuth'), href: admin_application_settings_path(anchor: 'js-signin-settings'), enabled: Gitlab::Auth.omniauth_enabled?) diff --git a/app/views/shared/issuable/_sidebar_assignees.html.haml b/app/views/shared/issuable/_sidebar_assignees.html.haml index dfb0e7ed297..e6b8e299e1c 100644 --- a/app/views/shared/issuable/_sidebar_assignees.html.haml +++ b/app/views/shared/issuable/_sidebar_assignees.html.haml @@ -11,7 +11,7 @@ = hidden_field_tag "#{issuable_type}[assignee_ids][]", 0, id: nil - else - assignees.each do |assignee| - = hidden_field_tag "#{issuable_type}[assignee_ids][]", assignee.id, id: nil, data: { avatar_url: assignee.avatar_url, name: assignee.name, username: assignee.username } + = hidden_field_tag "#{issuable_type}[assignee_ids][]", assignee.id, id: nil, data: assignee_sidebar_data(assignee, merge_request: @merge_request) - options = { toggle_class: 'js-user-search js-author-search', title: _('Assign to'), diff --git a/changelogs/unreleased/22959-remove-map-get-grid-breakpoints-xs-for-max-width.yml b/changelogs/unreleased/22959-remove-map-get-grid-breakpoints-xs-for-max-width.yml new file mode 100644 index 00000000000..12ef1fb23f2 --- /dev/null +++ b/changelogs/unreleased/22959-remove-map-get-grid-breakpoints-xs-for-max-width.yml @@ -0,0 +1,5 @@ +--- +title: Remove map-get($grid-breakpoints, xs) for max-width +merge_request: 17420 +author: Takuya Noguchi +type: other diff --git a/changelogs/unreleased/31386-fix-cannot-merge-icon-is-wrong.yml b/changelogs/unreleased/31386-fix-cannot-merge-icon-is-wrong.yml new file mode 100644 index 00000000000..442fc830dfa --- /dev/null +++ b/changelogs/unreleased/31386-fix-cannot-merge-icon-is-wrong.yml @@ -0,0 +1,5 @@ +--- +title: Fix cannot merge icon showing in dropdown for users who can merge +merge_request: 17306 +author: +type: fixed diff --git a/changelogs/unreleased/ee-16726-signup-not-disabled.yml b/changelogs/unreleased/ee-16726-signup-not-disabled.yml new file mode 100644 index 00000000000..eb0482f7046 --- /dev/null +++ b/changelogs/unreleased/ee-16726-signup-not-disabled.yml @@ -0,0 +1,5 @@ +--- +title: Fix signup link in admin area not being disabled +merge_request: 16726 +author: Illya Klymov +type: fixed diff --git a/changelogs/unreleased/jc-add-config-options-for-partial-clone.yml b/changelogs/unreleased/jc-add-config-options-for-partial-clone.yml new file mode 100644 index 00000000000..72a6339aa78 --- /dev/null +++ b/changelogs/unreleased/jc-add-config-options-for-partial-clone.yml @@ -0,0 +1,5 @@ +--- +title: Add allowFilter and allowAnySHA1InWant for partial clones +merge_request: 16850 +author: +type: added diff --git a/changelogs/unreleased/timeout-ci-includes-expansion.yml b/changelogs/unreleased/timeout-ci-includes-expansion.yml new file mode 100644 index 00000000000..07551655e2d --- /dev/null +++ b/changelogs/unreleased/timeout-ci-includes-expansion.yml @@ -0,0 +1,5 @@ +--- +title: Add timeout mechanism for CI config validation +merge_request: 16807 +author: +type: fixed diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index d74f80b3b33..3686b9b39e1 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -2289,6 +2289,10 @@ Nested includes allow you to compose a set of includes. A total of 50 includes is allowed. Duplicate includes are considered a configuration error. +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/28212) in GitLab 12.4. + +A hard limit of 30 seconds was set for resolving all files. + #### `include` examples Here are a few more `include` examples. diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 396467b47d1..2c624c9085e 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -104,6 +104,51 @@ document.addEventListener('DOMContentLoaded', () => new Vue({ })); ``` +#### Accessing feature flags + +Use Vue's [provide/inject](https://vuejs.org/v2/api/#provide-inject) mechanism +to make feature flags available to any descendant components in a Vue +application. The `glFeatures` object is already provided in `commons/vue.js`, so +only the mixin is required to utilize the flags: + +```javascript +// An arbitrary descendant component + +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; + +export default { + // ... + mixins: [glFeatureFlagsMixin()], + // ... + created() { + if (this.glFeatures.myFlag) { + // ... + } + }, +} +``` + +This approach has a few benefits: + +- Arbitrarily deeply nested components can opt-in and access the flag without + intermediate components being aware of it (c.f. passing the flag down via + props). +- Good testability, since the flag can be provided to `mount`/`shallowMount` + from `vue-test-utils` as easily as a prop. + + ```javascript + import { shallowMount } from '@vue/test-utils'; + + shallowMount(component, { + provide: { + glFeatures: { myFlag: true }, + }, + }); + ``` + +- No need to access a global variable, except in the application's + [entry point](#accessing-the-gl-object). + ### A folder for Components This folder holds all components that are specific of this new feature. diff --git a/doc/development/feature_flags/development.md b/doc/development/feature_flags/development.md index b338a191f76..929c9b1c71c 100644 --- a/doc/development/feature_flags/development.md +++ b/doc/development/feature_flags/development.md @@ -109,6 +109,9 @@ if ( gon.features.vimBindings ) { The name of the feature flag in JavaScript will always be camelCased, meaning that checking for `gon.features.vim_bindings` would not work. +See the [Vue guide](../fe_guide/vue.md#accessing-feature-flags) for details about +how to access feature flags in a Vue component. + ### Specs In the test environment `Feature.enabled?` is stubbed to always respond to `true`, diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index d5f0ddb0805..a5ec3e12b99 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -78,6 +78,10 @@ module API receive_max_input_size = Gitlab::CurrentSettings.receive_max_input_size.to_i if receive_max_input_size > 0 payload[:git_config_options] << "receive.maxInputSize=#{receive_max_input_size.megabytes}" + + if Feature.enabled?(:gitaly_upload_pack_filter, project) + payload[:git_config_options] << "uploadpack.allowFilter=true" << "uploadpack.allowAnySHA1InWant=true" + end end response_with_status(**payload) diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 668e4a5e246..342dcb2f784 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -7,6 +7,8 @@ module Gitlab # class Config ConfigError = Class.new(StandardError) + TIMEOUT_SECONDS = 30.seconds + TIMEOUT_MESSAGE = 'Resolving config took longer than expected' RESCUE_ERRORS = [ Gitlab::Config::Loader::FormatError, @@ -17,17 +19,17 @@ module Gitlab attr_reader :root def initialize(config, project: nil, sha: nil, user: nil) - @config = Config::Extendable - .new(build_config(config, project: project, sha: sha, user: user)) - .to_hash + @context = build_context(project: project, sha: sha, user: user) + + if Feature.enabled?(:ci_limit_yaml_expansion, project, default_enabled: true) + @context.set_deadline(TIMEOUT_SECONDS) + end + + @config = expand_config(config) @root = Entry::Root.new(@config) @root.compose! - rescue Gitlab::Config::Loader::Yaml::DataTooLargeError => e - Gitlab::Sentry.track_exception(e, extra: { user: user.inspect, project: project.inspect }) - raise Config::ConfigError, e.message - rescue *rescue_errors => e raise Config::ConfigError, e.message end @@ -61,18 +63,34 @@ module Gitlab private - def build_config(config, project:, sha:, user:) + def expand_config(config) + build_config(config) + + rescue Gitlab::Config::Loader::Yaml::DataTooLargeError => e + track_exception(e) + raise Config::ConfigError, e.message + + rescue Gitlab::Ci::Config::External::Context::TimeoutError => e + track_exception(e) + raise Config::ConfigError, TIMEOUT_MESSAGE + end + + def build_config(config) initial_config = Gitlab::Config::Loader::Yaml.new(config).load! + initial_config = Config::External::Processor.new(initial_config, @context).perform - process_external_files(initial_config, project: project, sha: sha, user: user) + Config::Extendable.new(initial_config).to_hash end - def process_external_files(config, project:, sha:, user:) - Config::External::Processor.new(config, + def build_context(project:, sha:, user:) + Config::External::Context.new( project: project, sha: sha || project&.repository&.root_ref_sha, - user: user, - expandset: Set.new).perform + user: user) + end + + def track_exception(error) + Gitlab::Sentry.track_exception(error, extra: @context.sentry_payload) end # Overriden in EE diff --git a/lib/gitlab/ci/config/external/context.rb b/lib/gitlab/ci/config/external/context.rb new file mode 100644 index 00000000000..bb4439cd069 --- /dev/null +++ b/lib/gitlab/ci/config/external/context.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module External + class Context + TimeoutError = Class.new(StandardError) + + attr_reader :project, :sha, :user + attr_reader :expandset, :execution_deadline + + def initialize(project: nil, sha: nil, user: nil) + @project = project + @sha = sha + @user = user + @expandset = Set.new + @execution_deadline = 0 + + yield self if block_given? + end + + def mutate(attrs = {}) + self.class.new(**attrs) do |ctx| + ctx.expandset = expandset + ctx.execution_deadline = execution_deadline + end + end + + def set_deadline(timeout_seconds) + @execution_deadline = current_monotonic_time + timeout_seconds.to_f + end + + def check_execution_time! + raise TimeoutError if execution_expired? + end + + def sentry_payload + { + user: user.inspect, + project: project.inspect + } + end + + protected + + attr_writer :expandset, :execution_deadline + + private + + def current_monotonic_time + Gitlab::Metrics::System.monotonic_time + end + + def execution_expired? + return false if execution_deadline.zero? + + current_monotonic_time > execution_deadline + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb index c56d33544ba..4684a9eb981 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -12,8 +12,6 @@ module Gitlab YAML_WHITELIST_EXTENSION = /.+\.(yml|yaml)$/i.freeze - Context = Struct.new(:project, :sha, :user, :expandset) - def initialize(params, context) @params = params @context = context @@ -69,11 +67,16 @@ module Gitlab end def validate! + validate_execution_time! validate_location! validate_content! if errors.none? validate_hash! if errors.none? end + def validate_execution_time! + context.check_execution_time! + end + def validate_location! if invalid_location_type? errors.push("Included file `#{location}` needs to be a string") @@ -95,11 +98,11 @@ module Gitlab end def expand_includes(hash) - External::Processor.new(hash, **expand_context).perform + External::Processor.new(hash, context.mutate(expand_context_attrs)).perform end - def expand_context - { project: nil, sha: nil, user: nil, expandset: context.expandset } + def expand_context_attrs + {} end end end diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb index cac321ec4a6..8cb1575a3e1 100644 --- a/lib/gitlab/ci/config/external/file/local.rb +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -6,6 +6,7 @@ module Gitlab module External module File class Local < Base + extend ::Gitlab::Utils::Override include Gitlab::Utils::StrongMemoize def initialize(params, context) @@ -34,11 +35,13 @@ module Gitlab context.project.repository.blob_data_at(context.sha, location) end - def expand_context - super.merge( + override :expand_context_attrs + def expand_context_attrs + { project: context.project, sha: context.sha, - user: context.user) + user: context.user + } end end end diff --git a/lib/gitlab/ci/config/external/file/project.rb b/lib/gitlab/ci/config/external/file/project.rb index b828f77835c..c7b49b495fa 100644 --- a/lib/gitlab/ci/config/external/file/project.rb +++ b/lib/gitlab/ci/config/external/file/project.rb @@ -6,11 +6,12 @@ module Gitlab module External module File class Project < Base + extend ::Gitlab::Utils::Override include Gitlab::Utils::StrongMemoize attr_reader :project_name, :ref_name - def initialize(params, context = {}) + def initialize(params, context) @location = params[:file] @project_name = params[:project] @ref_name = params[:ref] || 'HEAD' @@ -65,11 +66,13 @@ module Gitlab end end - def expand_context - super.merge( + override :expand_context_attrs + def expand_context_attrs + { project: project, sha: sha, - user: context.user) + user: context.user + } end end end diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb index aff5c5b9651..5f7694e2cc6 100644 --- a/lib/gitlab/ci/config/external/mapper.rb +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -21,14 +21,9 @@ module Gitlab DuplicateIncludesError = Class.new(Error) TooManyIncludesError = Class.new(Error) - def initialize(values, project:, sha:, user:, expandset:) - raise Error, 'Expanded needs to be `Set`' unless expandset.is_a?(Set) - + def initialize(values, context) @locations = Array.wrap(values.fetch(:include, [])) - @project = project - @sha = sha - @user = user - @expandset = expandset + @context = context end def process @@ -43,7 +38,9 @@ module Gitlab private - attr_reader :locations, :project, :sha, :user, :expandset + attr_reader :locations, :context + + delegate :expandset, to: :context # convert location if String to canonical form def normalize_location(location) @@ -68,11 +65,11 @@ module Gitlab end # We scope location to context, as this allows us to properly support - # relative incldues, and similarly looking relative in another project + # relative includes, and similarly looking relative in another project # does not trigger duplicate error scoped_location = location.merge( - context_project: project, - context_sha: sha) + context_project: context.project, + context_sha: context.sha) unless expandset.add?(scoped_location) raise DuplicateIncludesError, "Include `#{location.to_json}` was already included!" @@ -88,12 +85,6 @@ module Gitlab matching.first end - - def context - strong_memoize(:context) do - External::File::Base::Context.new(project, sha, user, expandset) - end - end end end end diff --git a/lib/gitlab/ci/config/external/processor.rb b/lib/gitlab/ci/config/external/processor.rb index 4a049ecae49..de69a1b1e8f 100644 --- a/lib/gitlab/ci/config/external/processor.rb +++ b/lib/gitlab/ci/config/external/processor.rb @@ -7,9 +7,9 @@ module Gitlab class Processor IncludeError = Class.new(StandardError) - def initialize(values, project:, sha:, user:, expandset:) + def initialize(values, context) @values = values - @external_files = External::Mapper.new(values, project: project, sha: sha, user: user, expandset: expandset).process + @external_files = External::Mapper.new(values, context).process @content = {} rescue External::Mapper::Error, OpenSSL::SSL::SSLError => e diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cd135f8db5b..0112d267c7c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15527,6 +15527,9 @@ msgstr "" msgid "The merge conflicts for this merge request have already been resolved. Please return to the merge request." msgstr "" +msgid "The merge request can now be merged." +msgstr "" + msgid "The name %{entryName} is already taken in this directory." msgstr "" diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 820ce159633..50666c8016d 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -483,10 +483,8 @@ describe Projects::EnvironmentsController do end shared_examples_for 'the default dashboard' do - all_dashboards = Feature.enabled?(:environment_metrics_show_multiple_dashboards) - it_behaves_like '200 response' - it_behaves_like 'includes all dashboards' if all_dashboards + it_behaves_like 'includes all dashboards' it 'is the default dashboard' do get :metrics_dashboard, params: environment_params(dashboard_params) @@ -618,16 +616,6 @@ describe Projects::EnvironmentsController do it_behaves_like 'the default dashboard' it_behaves_like 'dashboard can be specified' it_behaves_like 'dashboard can be embedded' - - context 'when multiple dashboards is disabled' do - before do - stub_feature_flags(environment_metrics_show_multiple_dashboards: false) - end - - it_behaves_like 'the default dashboard' - it_behaves_like 'dashboard cannot be specified' - it_behaves_like 'dashboard can be embedded' - end end describe 'GET #search' do diff --git a/spec/features/merge_request/user_edits_assignees_sidebar_spec.rb b/spec/features/merge_request/user_edits_assignees_sidebar_spec.rb new file mode 100644 index 00000000000..59c20f4ec6b --- /dev/null +++ b/spec/features/merge_request/user_edits_assignees_sidebar_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Merge request > User edits assignees sidebar', :js do + let(:project) { create(:project, :public, :repository) } + let(:protected_branch) { create(:protected_branch, :maintainers_can_push, name: 'master', project: project) } + let(:merge_request) { create(:merge_request, :simple, source_project: project, target_branch: protected_branch.name) } + + let(:users_find_limit) { 5 } + + # Insert more than limit so that response doesn't include assigned user + let(:project_developers) { Array.new(users_find_limit + 1) { create(:user).tap { |u| project.add_developer(u) } } } + let(:project_maintainers) { Array.new(users_find_limit + 1) { create(:user).tap { |u| project.add_maintainer(u) } } } + + # DOM finders to simplify and improve readability + let(:sidebar_assignee_block) { page.find('.js-issuable-sidebar .assignee') } + let(:sidebar_assignee_avatar_link) { sidebar_assignee_block.find_all('a').find { |a| a['href'].include? assignee.username } } + let(:sidebar_assignee_tooltip) { sidebar_assignee_avatar_link['data-original-title'] || '' } + let(:sidebar_assignee_dropdown_item) { sidebar_assignee_block.find(".dropdown-menu li[data-user-id=\"#{assignee.id}\"]") } + let(:sidebar_assignee_dropdown_tooltip) { sidebar_assignee_dropdown_item.find('a')['data-title'] || '' } + + before do + stub_const('Autocomplete::UsersFinder::LIMIT', users_find_limit) + + sign_in(project.owner) + + merge_request.assignees << assignee + + visit project_merge_request_path(project, merge_request) + + wait_for_requests + end + + shared_examples 'when assigned' do |expected_tooltip: ''| + it 'shows assignee name' do + expect(sidebar_assignee_block).to have_text(assignee.name) + end + + it "shows assignee tooltip '#{expected_tooltip}'" do + expect(sidebar_assignee_tooltip).to eql(expected_tooltip) + end + + context 'when edit is clicked' do + before do + sidebar_assignee_block.click_link('Edit') + + wait_for_requests + end + + it "shows assignee tooltip '#{expected_tooltip}" do + expect(sidebar_assignee_dropdown_tooltip).to eql(expected_tooltip) + end + end + end + + context 'when assigned to maintainer' do + let(:assignee) { project_maintainers.last } + + it_behaves_like 'when assigned', expected_tooltip: '' + end + + context 'when assigned to developer' do + let(:assignee) { project_developers.last } + + it_behaves_like 'when assigned', expected_tooltip: 'Cannot merge' + end +end diff --git a/spec/frontend/vue_shared/gl_feature_flags_plugin_spec.js b/spec/frontend/vue_shared/gl_feature_flags_plugin_spec.js new file mode 100644 index 00000000000..6ecc330b5af --- /dev/null +++ b/spec/frontend/vue_shared/gl_feature_flags_plugin_spec.js @@ -0,0 +1,42 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import GlFeatureFlags from '~/vue_shared/gl_feature_flags_plugin'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; + +const localVue = createLocalVue(); + +describe('GitLab Feature Flags Plugin', () => { + beforeEach(() => { + window.gon = { + features: { + aFeature: true, + bFeature: false, + }, + }; + + localVue.use(GlFeatureFlags); + }); + + it('should provide glFeatures to components', () => { + const component = { + template: `<span></span>`, + inject: ['glFeatures'], + }; + const wrapper = shallowMount(component, { localVue }); + expect(wrapper.vm.glFeatures).toEqual({ + aFeature: true, + bFeature: false, + }); + }); + + it('should integrate with the glFeatureMixin', () => { + const component = { + template: `<span></span>`, + mixins: [glFeatureFlagsMixin()], + }; + const wrapper = shallowMount(component, { localVue }); + expect(wrapper.vm.glFeatures).toEqual({ + aFeature: true, + bFeature: false, + }); + }); +}); diff --git a/spec/frontend/vue_shared/mixins/gl_feature_flags_mixin_spec.js b/spec/frontend/vue_shared/mixins/gl_feature_flags_mixin_spec.js new file mode 100644 index 00000000000..a3e3270a4e8 --- /dev/null +++ b/spec/frontend/vue_shared/mixins/gl_feature_flags_mixin_spec.js @@ -0,0 +1,36 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; + +const localVue = createLocalVue(); + +describe('GitLab Feature Flags Mixin', () => { + let wrapper; + + beforeEach(() => { + const gon = { + features: { + aFeature: true, + bFeature: false, + }, + }; + + const component = { + template: `<span></span>`, + mixins: [glFeatureFlagsMixin()], + }; + + wrapper = shallowMount(component, { + localVue, + provide: { + glFeatures: { ...(gon.features || {}) }, + }, + }); + }); + + it('should provide glFeatures to components', () => { + expect(wrapper.vm.glFeatures).toEqual({ + aFeature: true, + bFeature: false, + }); + }); +}); diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 583b8f90db9..2f67ea457a0 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -243,4 +243,32 @@ describe IssuablesHelper do end end end + + describe '#assignee_sidebar_data' do + let(:user) { create(:user) } + let(:merge_request) { nil } + subject { helper.assignee_sidebar_data(user, merge_request: merge_request) } + + it 'returns hash of assignee data' do + is_expected.to eql({ + avatar_url: user.avatar_url, + name: user.name, + username: user.username + }) + end + + context 'with merge_request' do + let(:merge_request) { build_stubbed(:merge_request) } + + where(can_merge: [true, false]) + + with_them do + before do + allow(merge_request).to receive(:can_be_merged_by?).and_return(can_merge) + end + + it { is_expected.to include({ can_merge: can_merge })} + end + end + end end diff --git a/spec/javascripts/create_cluster/gke_cluster/components/gke_project_id_dropdown_spec.js b/spec/javascripts/create_cluster/gke_cluster/components/gke_project_id_dropdown_spec.js index 809da3f9088..016ecfb35b8 100644 --- a/spec/javascripts/create_cluster/gke_cluster/components/gke_project_id_dropdown_spec.js +++ b/spec/javascripts/create_cluster/gke_cluster/components/gke_project_id_dropdown_spec.js @@ -4,6 +4,7 @@ import { createStore } from '~/create_cluster/gke_cluster/store'; import { SET_PROJECTS } from '~/create_cluster/gke_cluster/store/mutation_types'; import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import { emptyProjectMock, selectedProjectMock } from '../mock_data'; +import { gapi } from '../helpers'; const componentConfig = { docsUrl: 'https://console.cloud.google.com/home/dashboard', @@ -32,6 +33,16 @@ describe('GkeProjectIdDropdown', () => { let vm; let store; + let originalGapi; + beforeAll(() => { + originalGapi = window.gapi; + window.gapi = gapi(); + }); + + afterAll(() => { + window.gapi = originalGapi; + }); + beforeEach(() => { store = createStore(); vm = createComponent(store); diff --git a/spec/javascripts/create_cluster/gke_cluster/stores/actions_spec.js b/spec/javascripts/create_cluster/gke_cluster/stores/actions_spec.js index a7591cc38c7..7ceaeace82f 100644 --- a/spec/javascripts/create_cluster/gke_cluster/stores/actions_spec.js +++ b/spec/javascripts/create_cluster/gke_cluster/stores/actions_spec.js @@ -64,7 +64,15 @@ describe('GCP Cluster Dropdown Store Actions', () => { }); describe('async fetch methods', () => { - window.gapi = gapi(); + let originalGapi; + beforeAll(() => { + originalGapi = window.gapi; + window.gapi = gapi(); + }); + + afterAll(() => { + window.gapi = originalGapi; + }); describe('fetchProjects', () => { it('fetches projects from Google API', done => { diff --git a/spec/javascripts/monitoring/components/dashboard_spec.js b/spec/javascripts/monitoring/components/dashboard_spec.js index 6ce32d21f45..87aa4ba500d 100644 --- a/spec/javascripts/monitoring/components/dashboard_spec.js +++ b/spec/javascripts/monitoring/components/dashboard_spec.js @@ -527,7 +527,6 @@ describe('Dashboard', () => { component.$store.dispatch('monitoringDashboard/setFeatureFlags', { prometheusEndpoint: false, - multipleDashboardsEnabled: true, }); component.$store.commit( diff --git a/spec/javascripts/monitoring/store/actions_spec.js b/spec/javascripts/monitoring/store/actions_spec.js index 955a39e03a5..1bd74f59282 100644 --- a/spec/javascripts/monitoring/store/actions_spec.js +++ b/spec/javascripts/monitoring/store/actions_spec.js @@ -240,8 +240,6 @@ describe('Monitoring store actions', () => { const response = metricsDashboardResponse; response.all_dashboards = dashboardGitResponse; - state.multipleDashboardsEnabled = true; - receiveMetricsDashboardSuccess({ state, commit, dispatch }, { response, params }); expect(commit).toHaveBeenCalledWith(types.SET_ALL_DASHBOARDS, dashboardGitResponse); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_wip_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_wip_spec.js index 7b1d589dcf8..5844dad42ff 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_wip_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_wip_spec.js @@ -47,7 +47,7 @@ describe('Wip', () => { it('should make a request to service and handle response', done => { const vm = createComponent(); - spyOn(window, 'Flash').and.returnValue(true); + const flashSpy = spyOnDependency(WorkInProgress, 'createFlash').and.returnValue(true); spyOn(eventHub, '$emit'); spyOn(vm.service, 'removeWIP').and.returnValue( new Promise(resolve => { @@ -61,10 +61,7 @@ describe('Wip', () => { setTimeout(() => { expect(vm.isMakingRequest).toBeTruthy(); expect(eventHub.$emit).toHaveBeenCalledWith('UpdateWidgetData', mrObj); - expect(window.Flash).toHaveBeenCalledWith( - 'The merge request can now be merged.', - 'notice', - ); + expect(flashSpy).toHaveBeenCalledWith('The merge request can now be merged.', 'notice'); done(); }, 333); }); diff --git a/spec/lib/gitlab/ci/config/external/context_spec.rb b/spec/lib/gitlab/ci/config/external/context_spec.rb new file mode 100644 index 00000000000..610646ca85a --- /dev/null +++ b/spec/lib/gitlab/ci/config/external/context_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +describe Gitlab::Ci::Config::External::Context do + let(:project) { double('Project') } + let(:user) { double('User') } + let(:sha) { '12345' } + let(:attributes) { { project: project, user: user, sha: sha } } + + subject(:subject) { described_class.new(**attributes) } + + describe 'attributes' do + context 'with values' do + it { is_expected.to have_attributes(**attributes) } + it { expect(subject.expandset).to eq(Set.new) } + it { expect(subject.execution_deadline).to eq(0) } + end + + context 'without values' do + let(:attributes) { { project: nil, user: nil, sha: nil } } + + it { is_expected.to have_attributes(**attributes) } + it { expect(subject.expandset).to eq(Set.new) } + it { expect(subject.execution_deadline).to eq(0) } + end + end + + describe '#set_deadline' do + let(:stubbed_time) { 1 } + + before do + allow(subject).to receive(:current_monotonic_time).and_return(stubbed_time) + end + + context 'with a float value' do + let(:timeout_seconds) { 10.5.seconds } + + it 'updates execution_deadline' do + expect { subject.set_deadline(timeout_seconds) } + .to change { subject.execution_deadline } + .to(timeout_seconds + stubbed_time) + end + end + + context 'with nil as a value' do + let(:timeout_seconds) {} + + it 'updates execution_deadline' do + expect { subject.set_deadline(timeout_seconds) } + .to change { subject.execution_deadline } + .to(stubbed_time) + end + end + end + + describe '#check_execution_time!' do + before do + allow(subject).to receive(:current_monotonic_time).and_return(stubbed_time) + allow(subject).to receive(:execution_deadline).and_return(stubbed_deadline) + end + + context 'when execution is expired' do + let(:stubbed_time) { 2 } + let(:stubbed_deadline) { 1 } + + it 'raises an error' do + expect { subject.check_execution_time! } + .to raise_error(described_class::TimeoutError) + end + end + + context 'when execution is not expired' do + let(:stubbed_time) { 1 } + let(:stubbed_deadline) { 2 } + + it 'does not raises any errors' do + expect { subject.check_execution_time! }.not_to raise_error + end + end + + context 'without setting a deadline' do + let(:stubbed_time) { 2 } + let(:stubbed_deadline) { 1 } + + before do + allow(subject).to receive(:execution_deadline).and_call_original + end + + it 'does not raises any errors' do + expect { subject.check_execution_time! }.not_to raise_error + end + end + end + + describe '#mutate' do + shared_examples 'a mutated context' do + let(:mutated) { subject.mutate(new_attributes) } + + before do + subject.expandset << :a_file + subject.set_deadline(15.seconds) + end + + it { expect(mutated).not_to eq(subject) } + it { expect(mutated).to be_a(described_class) } + it { expect(mutated).to have_attributes(new_attributes) } + it { expect(mutated.expandset).to eq(subject.expandset) } + it { expect(mutated.execution_deadline).to eq(mutated.execution_deadline) } + end + + context 'with attributes' do + let(:new_attributes) { { project: double, user: double, sha: '56789' } } + + it_behaves_like 'a mutated context' + end + + context 'without attributes' do + let(:new_attributes) { {} } + + it_behaves_like 'a mutated context' + end + end + + describe '#sentry_payload' do + it { expect(subject.sentry_payload).to match(a_hash_including(:project, :user)) } + end +end diff --git a/spec/lib/gitlab/ci/config/external/file/base_spec.rb b/spec/lib/gitlab/ci/config/external/file/base_spec.rb index af995f4869a..d472d6527e2 100644 --- a/spec/lib/gitlab/ci/config/external/file/base_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/base_spec.rb @@ -1,13 +1,14 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' describe Gitlab::Ci::Config::External::File::Base do - let(:context) { described_class::Context.new(nil, 'HEAD', nil, Set.new) } + let(:context_params) { { sha: 'HEAD' } } + let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:test_class) do Class.new(described_class) do - def initialize(params, context = {}) + def initialize(params, context) @location = params super @@ -20,6 +21,9 @@ describe Gitlab::Ci::Config::External::File::Base do before do allow_any_instance_of(test_class) .to receive(:content).and_return('key: value') + + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) end describe '#matching?' do diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index 9451db9522a..95f0c93e758 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -7,10 +7,17 @@ describe Gitlab::Ci::Config::External::File::Local do set(:user) { create(:user) } let(:sha) { '12345' } - let(:context) { described_class::Context.new(project, sha, user, Set.new) } + let(:context_params) { { project: project, sha: sha, user: user } } + let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } + let(:params) { { local: location } } let(:local_file) { described_class.new(params, context) } + before do + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) + end + describe '#matching?' do context 'when a local is specified' do let(:params) { { local: 'file' } } @@ -109,7 +116,7 @@ describe Gitlab::Ci::Config::External::File::Local do describe '#expand_context' do let(:location) { 'location.yml' } - subject { local_file.send(:expand_context) } + subject { local_file.send(:expand_context_attrs) } it 'inherits project, user and sha' do is_expected.to include(user: user, project: project, sha: sha) diff --git a/spec/lib/gitlab/ci/config/external/file/project_spec.rb b/spec/lib/gitlab/ci/config/external/file/project_spec.rb index 4acb4f324d3..dd869c227a1 100644 --- a/spec/lib/gitlab/ci/config/external/file/project_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/project_spec.rb @@ -8,11 +8,15 @@ describe Gitlab::Ci::Config::External::File::Project do set(:user) { create(:user) } let(:context_user) { user } - let(:context) { described_class::Context.new(context_project, '12345', context_user, Set.new) } + let(:context_params) { { project: context_project, sha: '12345', user: context_user } } + let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:project_file) { described_class.new(params, context) } before do project.add_developer(user) + + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) end describe '#matching?' do @@ -145,7 +149,7 @@ describe Gitlab::Ci::Config::External::File::Project do describe '#expand_context' do let(:params) { { file: 'file.yml', project: project.full_path, ref: 'master' } } - subject { project_file.send(:expand_context) } + subject { project_file.send(:expand_context_attrs) } it 'inherits user, and target project and sha' do is_expected.to include(user: user, project: project, sha: project.commit('master').id) diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index 4a097b59216..08db00dda9d 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -5,7 +5,8 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Remote do include StubRequests - let(:context) { described_class::Context.new(nil, '12345', nil, Set.new) } + let(:context_params) { { sha: '12345' } } + let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:params) { { remote: location } } let(:remote_file) { described_class.new(params, context) } let(:location) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } @@ -19,6 +20,11 @@ describe Gitlab::Ci::Config::External::File::Remote do HEREDOC end + before do + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) + end + describe '#matching?' do context 'when a remote is specified' do let(:params) { { remote: 'http://remote' } } @@ -187,10 +193,10 @@ describe Gitlab::Ci::Config::External::File::Remote do describe '#expand_context' do let(:params) { { remote: 'http://remote' } } - subject { remote_file.send(:expand_context) } + subject { remote_file.send(:expand_context_attrs) } it 'drops all parameters' do - is_expected.to include(user: nil, project: nil, sha: nil) + is_expected.to be_empty end end end diff --git a/spec/lib/gitlab/ci/config/external/file/template_spec.rb b/spec/lib/gitlab/ci/config/external/file/template_spec.rb index 1609b8fd66b..164b5800abf 100644 --- a/spec/lib/gitlab/ci/config/external/file/template_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/template_spec.rb @@ -6,12 +6,18 @@ describe Gitlab::Ci::Config::External::File::Template do set(:project) { create(:project) } set(:user) { create(:user) } - let(:context) { described_class::Context.new(project, '12345', user, Set.new) } + let(:context_params) { { project: project, sha: '12345', user: user } } + let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:template) { 'Auto-DevOps.gitlab-ci.yml' } let(:params) { { template: template } } let(:template_file) { described_class.new(params, context) } + before do + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) + end + describe '#matching?' do context 'when a template is specified' do let(:params) { { template: 'some-template' } } @@ -97,10 +103,10 @@ describe Gitlab::Ci::Config::External::File::Template do describe '#expand_context' do let(:location) { 'location.yml' } - subject { template_file.send(:expand_context) } + subject { template_file.send(:expand_context_attrs) } it 'drops all parameters' do - is_expected.to include(user: nil, project: nil, sha: nil) + is_expected.to be_empty end end end diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index 43708852594..8d09aa47f12 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -11,7 +11,8 @@ describe Gitlab::Ci::Config::External::Mapper do let(:local_file) { '/lib/gitlab/ci/templates/non-existent-file.yml' } let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } let(:template_file) { 'Auto-DevOps.gitlab-ci.yml' } - let(:expandset) { Set.new } + let(:context_params) { { project: project, sha: '123456', user: user } } + let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:file_content) do <<~HEREDOC @@ -21,10 +22,13 @@ describe Gitlab::Ci::Config::External::Mapper do before do stub_full_request(remote_url).to_return(body: file_content) + + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) end describe '#process' do - subject { described_class.new(values, project: project, sha: '123456', user: user, expandset: expandset).process } + subject { described_class.new(values, context).process } context "when single 'include' keyword is defined" do context 'when the string is a local file' do diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 3b1a1e804f0..bb2d3f66972 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -9,12 +9,16 @@ describe Gitlab::Ci::Config::External::Processor do set(:another_project) { create(:project, :repository) } set(:user) { create(:user) } - let(:expandset) { Set.new } let(:sha) { '12345' } - let(:processor) { described_class.new(values, project: project, sha: '12345', user: user, expandset: expandset) } + let(:context_params) { { project: project, sha: sha, user: user } } + let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } + let(:processor) { described_class.new(values, context) } before do project.add_developer(user) + + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) end describe "#perform" do diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 839b4f9261d..68c38644b5c 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -7,6 +7,11 @@ describe Gitlab::Ci::Config do set(:user) { create(:user) } + before do + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) + end + let(:config) do described_class.new(yml, project: nil, sha: nil, user: nil) end @@ -303,6 +308,49 @@ describe Gitlab::Ci::Config do end end + context "when it takes too long to evaluate includes" do + before do + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) + .and_call_original + + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:set_deadline) + .with(described_class::TIMEOUT_SECONDS) + .and_call_original + + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:execution_expired?) + .and_return(true) + end + + it 'raises error TimeoutError' do + expect(Gitlab::Sentry).to receive(:track_exception) + + expect { config }.to raise_error( + described_class::ConfigError, + 'Resolving config took longer than expected' + ) + end + end + + context 'when context expansion timeout is disabled' do + before do + allow_any_instance_of(Gitlab::Ci::Config::External::Context) + .to receive(:check_execution_time!) + .and_call_original + + allow(Feature) + .to receive(:enabled?) + .with(:ci_limit_yaml_expansion, project, default_enabled: true) + .and_return(false) + end + + it 'does not raises errors' do + expect { config }.not_to raise_error + end + end + describe 'external file version' do context 'when external local file SHA is defined' do it 'is using a defined value' do diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index a56527073c7..4d13b32d4a1 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -403,12 +403,30 @@ describe API::Internal::Base do end context 'when receive_max_input_size has been updated' do - it 'returns custom git config' do + before do allow(Gitlab::CurrentSettings).to receive(:receive_max_input_size) { 1 } + end + it 'returns custom git config' do push(key, project) expect(json_response["git_config_options"]).to be_present + expect(json_response["git_config_options"]).to include("uploadpack.allowFilter=true") + expect(json_response["git_config_options"]).to include("uploadpack.allowAnySHA1InWant=true") + end + + context 'when gitaly_upload_pack_filter feature flag is disabled' do + before do + stub_feature_flags(gitaly_upload_pack_filter: { enabled: false, thing: project }) + end + + it 'does not include allowFilter and allowAnySha1InWant in the git config options' do + push(key, project) + + expect(json_response["git_config_options"]).to be_present + expect(json_response["git_config_options"]).not_to include("uploadpack.allowFilter=true") + expect(json_response["git_config_options"]).not_to include("uploadpack.allowAnySHA1InWant=true") + end end end |