diff options
151 files changed, 2966 insertions, 837 deletions
diff --git a/.gitignore b/.gitignore index 3ffe4263c4f..fcbb4c352a9 100644 --- a/.gitignore +++ b/.gitignore @@ -61,11 +61,9 @@ eslint-report.html /shared/artifacts/ /rails_best_practices_output.html /tags -/tmp/* /vendor/bundle/* /vendor/gitaly-ruby /builds* -/shared/* /.gitlab_workhorse_secret /webpack-report/ /knapsack/ @@ -73,7 +71,6 @@ eslint-report.html /locale/**/LC_MESSAGES /locale/**/*.time_stamp /.rspec -/plugins/* /.gitlab_pages_secret /.gitlab_smime_key /.gitlab_smime_cert diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 5b5527284d3..b3b6beb1068 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -5,6 +5,7 @@ stages: - prepare - quick-test - test + - review-prepare - review - qa - post-test diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 0720ea3e056..a20215694c0 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -13,7 +13,9 @@ - .default-before_script - .except-docs image: dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.6.3-git-2.22-chrome-73.0-node-12.x-yarn-1.16-graphicsmagick-1.3.33-docker-18.06.1 + stage: test dependencies: ["setup-test-env"] + needs: ["setup-test-env"] services: - docker:19.03.0-dind variables: @@ -42,10 +44,6 @@ - time scripts/build_assets_image - scripts/clean-old-cached-assets - rm -f /etc/apt/sources.list.d/google*.list # We don't need to update Chrome here - # Play dependent manual jobs - - install_api_client_dependencies_with_apt - - play_job "review-build-cng" || true # this job might not exist so ignore the failure if it cannot be played - - play_job "schedule:review-build-cng" || true # this job might not exist so ignore the failure if it cannot be played only: - /.+/@gitlab-org/gitlab-ce - /.+/@gitlab-org/gitlab-ee @@ -186,7 +184,8 @@ jest: - .default-retry - .default-cache - .except-docs - dependencies: ["setup-test-env"] + dependencies: [] + stage: test variables: SETUP_DB: "false" before_script: diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 2e8b197829b..878be25c39b 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -165,7 +165,9 @@ downtime_check: - /^[\d-]+-stable(-ee)?$/ - /(^docs[\/-].+|.+-docs$)/ - /(^qa[\/-].*|.*-qa$)/ + stage: test dependencies: ["setup-test-env"] + needs: ["setup-test-env"] ee_compat_check: extends: .rake-exec @@ -195,7 +197,9 @@ db:migrate:reset: - .default-before_script - .use-pg - .except-docs-qa + stage: test dependencies: ["setup-test-env"] + needs: ["setup-test-env"] script: - bundle exec rake db:migrate:reset @@ -207,7 +211,9 @@ db:check-schema: - .default-before_script - .use-pg - .except-docs-qa + stage: test dependencies: ["setup-test-env"] + needs: ["setup-test-env"] script: - source scripts/schema_changed.sh @@ -219,7 +225,9 @@ db:migrate-from-v11.11.0: - .default-before_script - .use-pg - .except-docs-qa + stage: test dependencies: ["setup-test-env"] + needs: ["setup-test-env"] variables: SETUP_DB: "false" script: @@ -248,7 +256,9 @@ db:rollback: - .default-before_script - .use-pg - .except-docs-qa + stage: test dependencies: ["setup-test-env"] + needs: ["setup-test-env"] script: - bundle exec rake db:migrate VERSION=20180101160629 - bundle exec rake db:migrate SKIP_SCHEMA_VERSION_CHECK=true @@ -261,7 +271,9 @@ gitlab:setup: - .default-before_script - .use-pg - .except-docs-qa + stage: test dependencies: ["setup-test-env"] + needs: ["setup-test-env"] variables: SETUP_DB: "false" script: diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 6695404653c..6f1505b5c0d 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -53,8 +53,7 @@ build-qa-image: .review-build-cng-base: image: ruby:2.6-alpine - stage: test - when: manual + stage: review-prepare before_script: - source scripts/utils.sh - install_api_client_dependencies_with_apk @@ -67,17 +66,20 @@ review-build-cng: extends: - .review-build-cng-base - .review-only + needs: ["gitlab:assets:compile pull-cache"] schedule:review-build-cng: extends: - .review-build-cng-base - .review-schedules-only + needs: ["gitlab:assets:compile"] review-deploy: extends: .review-base allow_failure: true retry: 1 stage: review + needs: ["review-build-cng"] variables: HOST_SUFFIX: "${CI_ENVIRONMENT_SLUG}" DOMAIN: "-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN}" diff --git a/.gitlab/ci/setup.gitlab-ci.yml b/.gitlab/ci/setup.gitlab-ci.yml index d9384780356..cd7ca4d30bf 100644 --- a/.gitlab/ci/setup.gitlab-ci.yml +++ b/.gitlab/ci/setup.gitlab-ci.yml @@ -7,7 +7,9 @@ cache gems: - .default-cache - .default-before_script - .except-docs + stage: test dependencies: ["setup-test-env"] + needs: ["setup-test-env"] variables: SETUP_DB: "false" script: diff --git a/Dangerfile b/Dangerfile index 094d55e8652..228190cd530 100644 --- a/Dangerfile +++ b/Dangerfile @@ -1,23 +1,12 @@ # frozen_string_literal: true + +require_relative 'lib/gitlab_danger' + danger.import_plugin('danger/plugins/helper.rb') danger.import_plugin('danger/plugins/roulette.rb') unless helper.release_automation? - danger.import_dangerfile(path: 'danger/metadata') - danger.import_dangerfile(path: 'danger/changes_size') - danger.import_dangerfile(path: 'danger/changelog') - danger.import_dangerfile(path: 'danger/specs') - danger.import_dangerfile(path: 'danger/gemfile') - danger.import_dangerfile(path: 'danger/database') - danger.import_dangerfile(path: 'danger/documentation') - danger.import_dangerfile(path: 'danger/frozen_string') - danger.import_dangerfile(path: 'danger/commit_messages') - danger.import_dangerfile(path: 'danger/duplicate_yarn_dependencies') - danger.import_dangerfile(path: 'danger/prettier') - danger.import_dangerfile(path: 'danger/eslint') - danger.import_dangerfile(path: 'danger/roulette') - danger.import_dangerfile(path: 'danger/single_codebase') - danger.import_dangerfile(path: 'danger/gitlab_ui_wg') - danger.import_dangerfile(path: 'danger/ce_ee_vue_templates') - danger.import_dangerfile(path: 'danger/only_documentation') + GitlabDanger.new(helper.gitlab_helper).rule_names.each do |file| + danger.import_dangerfile(path: File.join('danger', file)) + end end @@ -318,6 +318,7 @@ end group :development do gem 'foreman', '~> 0.84.0' gem 'brakeman', '~> 4.2', require: false + gem 'danger', '~> 6.0', require: false gem 'letter_opener_web', '~> 1.3.4' gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false diff --git a/Gemfile.lock b/Gemfile.lock index 6add217bc32..d787b5c0569 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -140,9 +140,15 @@ GEM numerizer (~> 0.1.1) chunky_png (1.3.5) citrus (3.0.2) + claide (1.0.3) + claide-plugins (0.9.2) + cork + nap + open4 (~> 1.3) coderay (1.1.2) coercible (1.0.0) descendants_tracker (~> 0.0.1) + colored2 (3.1.2) commonmarker (0.17.13) ruby-enum (~> 0.5) concord (0.1.5) @@ -151,6 +157,8 @@ GEM concurrent-ruby (1.1.5) connection_pool (2.2.2) contracts (0.11.0) + cork (0.3.0) + colored2 (~> 3.1) crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.4) @@ -158,6 +166,19 @@ GEM css_parser (1.5.0) addressable daemons (1.2.6) + danger (6.0.9) + claide (~> 1.0) + claide-plugins (>= 0.9.2) + colored2 (~> 3.1) + cork (~> 0.1) + faraday (~> 0.9) + faraday-http-cache (~> 2.0) + git (~> 1.5) + kramdown (~> 2.0) + kramdown-parser-gfm (~> 1.0) + no_proxy_fix + octokit (~> 4.7) + terminal-table (~> 1) database_cleaner (1.7.0) debug_inspector (0.0.3) debugger-ruby_core_source (1.3.8) @@ -227,6 +248,8 @@ GEM railties (>= 3.0.0) faraday (0.12.2) multipart-post (>= 1.2, < 3) + faraday-http-cache (2.0.0) + faraday (~> 0.8) faraday_middleware (0.12.2) faraday (>= 0.7.4, < 1.0) faraday_middleware-multi_json (0.0.6) @@ -308,6 +331,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) + git (1.5.0) gitaly (1.58.0) grpc (~> 1.0) github-markup (1.7.0) @@ -478,6 +502,9 @@ GEM kgio (2.11.2) knapsack (1.17.0) rake + kramdown (2.1.0) + kramdown-parser-gfm (1.1.0) + kramdown (~> 2.0) kubeclient (4.2.2) http (~> 3.0) recursive-open-struct (~> 1.0, >= 1.0.4) @@ -535,10 +562,12 @@ GEM mustermann-grape (1.0.0) mustermann (~> 1.0.0) nakayoshi_fork (0.0.4) + nap (1.1.0) net-ldap (0.16.0) net-ssh (5.2.0) netrc (0.11.0) nio4r (2.3.1) + no_proxy_fix (0.1.2) nokogiri (1.10.4) mini_portile2 (~> 2.4.0) nokogumbo (1.5.0) @@ -615,6 +644,7 @@ GEM addressable (~> 2.5) omniauth (~> 1.3) openid_connect (~> 1.1) + open4 (1.3.4) openid_connect (1.1.6) activemodel attr_required (>= 1.0.0) @@ -926,6 +956,8 @@ GEM ffi sysexits (1.2.0) temple (0.8.0) + terminal-table (1.8.0) + unicode-display_width (~> 1.1, >= 1.1.1) test-prof (0.2.5) text (1.3.1) thin (1.7.2) @@ -1055,6 +1087,7 @@ DEPENDENCIES concurrent-ruby (~> 1.1) connection_pool (~> 2.0) creole (~> 0.5.0) + danger (~> 6.0) database_cleaner (~> 1.7.0) deckar01-task_list (= 2.2.0) default_value_for (~> 3.2.0) diff --git a/app/assets/javascripts/boards/components/board_new_issue.vue b/app/assets/javascripts/boards/components/board_new_issue.vue index f9284266b72..f9a08f151c5 100644 --- a/app/assets/javascripts/boards/components/board_new_issue.vue +++ b/app/assets/javascripts/boards/components/board_new_issue.vue @@ -2,9 +2,9 @@ import $ from 'jquery'; import { GlButton } from '@gitlab/ui'; import { getMilestone } from 'ee_else_ce/boards/boards_util'; +import ListIssue from 'ee_else_ce/boards/models/issue'; import eventHub from '../eventhub'; import ProjectSelect from './project_select.vue'; -import ListIssue from '../models/issue'; import boardsStore from '../stores/boards_store'; export default { @@ -54,6 +54,9 @@ export default { const assignees = this.list.assignee ? [this.list.assignee] : []; const milestone = getMilestone(this.list); + const { weightFeatureAvailable } = boardsStore; + const { weight } = weightFeatureAvailable ? boardsStore.state.currentBoard : {}; + const issue = new ListIssue({ title: this.title, labels, @@ -61,6 +64,7 @@ export default { assignees, milestone, project_id: this.selectedProject.id, + weight, }); eventHub.$emit(`scroll-board-list-${this.list.id}`); diff --git a/app/assets/javascripts/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue index 970f5a7b297..27959898fb7 100644 --- a/app/assets/javascripts/clusters/components/applications.vue +++ b/app/assets/javascripts/clusters/components/applications.vue @@ -397,7 +397,6 @@ export default { </div> </application-row> <application-row - v-if="isProjectCluster" id="jupyter" :logo-url="jupyterhubLogo" :title="applications.jupyter.title" @@ -459,7 +458,6 @@ export default { </div> </application-row> <application-row - v-if="isProjectCluster" id="knative" :logo-url="knativeLogo" :title="applications.knative.title" diff --git a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue index 43ae54133af..b1d568532a6 100644 --- a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue +++ b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue @@ -38,6 +38,10 @@ export default { type: String, required: true, }, + userCanEnableErrorTracking: { + type: Boolean, + required: true, + }, }, computed: { ...mapState(['errors', 'externalUrl', 'loading']), @@ -111,14 +115,26 @@ export default { </gl-table> </div> </div> - <div v-else> + <div v-else-if="userCanEnableErrorTracking"> <gl-empty-state :title="__('Get started with error tracking')" - :description="__('Monitor your errors by integrating with Sentry')" + :description="__('Monitor your errors by integrating with Sentry.')" :primary-button-text="__('Enable error tracking')" :primary-button-link="enableErrorTrackingLink" :svg-path="illustrationPath" /> </div> + <div v-else> + <gl-empty-state :title="__('Get started with error tracking')" :svg-path="illustrationPath"> + <template #description> + <div> + <span>{{ __('Monitor your errors by integrating with Sentry.') }}</span> + <a href="/help/user/project/operations/error_tracking.html"> + {{ __('More information') }} + </a> + </div> + </template> + </gl-empty-state> + </div> </div> </template> diff --git a/app/assets/javascripts/error_tracking/index.js b/app/assets/javascripts/error_tracking/index.js index 3d609448efe..073e2c8f1c7 100644 --- a/app/assets/javascripts/error_tracking/index.js +++ b/app/assets/javascripts/error_tracking/index.js @@ -14,9 +14,10 @@ export default () => { render(createElement) { const domEl = document.querySelector(this.$options.el); const { indexPath, enableErrorTrackingLink, illustrationPath } = domEl.dataset; - let { errorTrackingEnabled } = domEl.dataset; + let { errorTrackingEnabled, userCanEnableErrorTracking } = domEl.dataset; errorTrackingEnabled = parseBoolean(errorTrackingEnabled); + userCanEnableErrorTracking = parseBoolean(userCanEnableErrorTracking); return createElement('error-tracking-list', { props: { @@ -24,6 +25,7 @@ export default () => { enableErrorTrackingLink, errorTrackingEnabled, illustrationPath, + userCanEnableErrorTracking, }, }); }, diff --git a/app/assets/javascripts/pipelines/components/graph/graph_component.vue b/app/assets/javascripts/pipelines/components/graph/graph_component.vue index ba0dea626dc..27c1b639889 100644 --- a/app/assets/javascripts/pipelines/components/graph/graph_component.vue +++ b/app/assets/javascripts/pipelines/components/graph/graph_component.vue @@ -20,6 +20,9 @@ export default { <stage-column-component v-for="(stage, index) in graph" :key="stage.name" + :class="{ + 'append-right-48': shouldAddRightMargin(index), + }" :title="capitalizeStageName(stage.name)" :groups="stage.groups" :stage-connector-class="stageConnectorClass(index, stage)" diff --git a/app/assets/javascripts/pipelines/mixins/graph_component_mixin.js b/app/assets/javascripts/pipelines/mixins/graph_component_mixin.js index 66e9476dadf..f383a4b3368 100644 --- a/app/assets/javascripts/pipelines/mixins/graph_component_mixin.js +++ b/app/assets/javascripts/pipelines/mixins/graph_component_mixin.js @@ -40,5 +40,15 @@ export default { refreshPipelineGraph() { this.$emit('refreshPipelineGraph'); }, + /** + * CSS class is applied: + * - if pipeline graph contains only one stage column component + * + * @param {number} index + * @returns {boolean} + */ + shouldAddRightMargin(index) { + return !(index === this.graph.length - 1); + }, }, }; diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index e9218dcec67..b95978b6966 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -395,6 +395,7 @@ img.emoji { .prepend-left-default { margin-left: $gl-padding; } .prepend-left-20 { margin-left: 20px; } .prepend-left-32 { margin-left: 32px; } +.prepend-left-64 { margin-left: 64px; } .append-right-4 { margin-right: 4px; } .append-right-5 { margin-right: 5px; } .append-right-8 { margin-right: 8px; } @@ -402,6 +403,8 @@ img.emoji { .append-right-15 { margin-right: 15px; } .append-right-default { margin-right: $gl-padding; } .append-right-20 { margin-right: 20px; } +.append-right-32 { margin-right: 32px; } +.append-right-48 { margin-right: 48px; } .prepend-right-32 { margin-right: 32px; } .append-bottom-0 { margin-bottom: 0; } .append-bottom-4 { margin-bottom: $gl-padding-4; } diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 5f4db37c317..d4bd5b1b7dc 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -476,10 +476,6 @@ display: inline-block; vertical-align: top; - &:not(:last-child) { - margin-right: 44px; - } - &.left-margin { &:not(:first-child) { margin-left: 44px; diff --git a/app/controllers/projects/settings/operations_controller.rb b/app/controllers/projects/settings/operations_controller.rb index 5cfb0ac307d..ec89bb89edc 100644 --- a/app/controllers/projects/settings/operations_controller.rb +++ b/app/controllers/projects/settings/operations_controller.rb @@ -3,7 +3,7 @@ module Projects module Settings class OperationsController < Projects::ApplicationController - before_action :authorize_update_environment! + before_action :authorize_admin_operations! helper_method :error_tracking_setting diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index b735f9ff3b8..8ed6ff56e2b 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -193,15 +193,30 @@ class IssuableFinder projects = if current_user && params[:authorized_only].presence && !current_user_related? current_user.authorized_projects(min_access_level) - elsif group - find_group_projects else - Project.public_or_visible_to_user(current_user, min_access_level) + projects_public_or_visible_to_user end @projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil) # rubocop: disable CodeReuse/ActiveRecord end + def projects_public_or_visible_to_user + projects = + if group + if params[:projects] + find_group_projects.id_in(params[:projects]) + else + find_group_projects + end + elsif params[:projects] + Project.id_in(params[:projects]) + else + Project + end + + projects.public_or_visible_to_user(current_user, min_access_level) + end + def find_group_projects return Project.none unless group @@ -209,7 +224,7 @@ class IssuableFinder Project.where(namespace_id: group.self_and_descendants) # rubocop: disable CodeReuse/ActiveRecord else group.projects - end.public_or_visible_to_user(current_user, min_access_level) + end end def search diff --git a/app/graphql/types/permission_types/project.rb b/app/graphql/types/permission_types/project.rb index e9a4ea9157b..993d33c4fc2 100644 --- a/app/graphql/types/permission_types/project.rb +++ b/app/graphql/types/permission_types/project.rb @@ -16,7 +16,7 @@ module Types :create_deployment, :push_to_delete_protected_branch, :admin_wiki, :admin_project, :update_pages, :admin_remote_mirror, :create_label, :update_wiki, :destroy_wiki, - :create_pages, :destroy_pages, :read_pages_content + :create_pages, :destroy_pages, :read_pages_content, :admin_operations end end end diff --git a/app/helpers/projects/error_tracking_helper.rb b/app/helpers/projects/error_tracking_helper.rb index 6daf2e21ca2..fd1222a1dfb 100644 --- a/app/helpers/projects/error_tracking_helper.rb +++ b/app/helpers/projects/error_tracking_helper.rb @@ -1,12 +1,13 @@ # frozen_string_literal: true module Projects::ErrorTrackingHelper - def error_tracking_data(project) + def error_tracking_data(current_user, project) error_tracking_enabled = !!project.error_tracking_setting&.enabled? { 'index-path' => project_error_tracking_index_path(project, format: :json), + 'user-can-enable-error-tracking' => can?(current_user, :admin_operations, project).to_s, 'enable-error-tracking-link' => project_settings_operations_path(project), 'error-tracking-enabled' => error_tracking_enabled.to_s, 'illustration-path' => image_path('illustrations/cluster_popover.svg') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index d620959b538..d2271c1335c 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -23,6 +23,7 @@ module Ci belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline' belongs_to :pipeline_schedule, class_name: 'Ci::PipelineSchedule' belongs_to :merge_request, class_name: 'MergeRequest' + belongs_to :external_pull_request has_internal_id :iid, scope: :project, presence: false, init: ->(s) do s&.project&.all_pipelines&.maximum(:iid) || s&.project&.all_pipelines&.count @@ -64,6 +65,11 @@ module Ci validates :merge_request, presence: { if: :merge_request_event? } validates :merge_request, absence: { unless: :merge_request_event? } validates :tag, inclusion: { in: [false], if: :merge_request_event? } + + validates :external_pull_request, presence: { if: :external_pull_request_event? } + validates :external_pull_request, absence: { unless: :external_pull_request_event? } + validates :tag, inclusion: { in: [false], if: :external_pull_request_event? } + validates :status, presence: { unless: :importing? } validate :valid_commit_sha, unless: :importing? validates :source, exclusion: { in: %w(unknown), unless: :importing? }, on: :create @@ -683,6 +689,10 @@ module Ci variables.append(key: 'CI_MERGE_REQUEST_TARGET_BRANCH_SHA', value: target_sha.to_s) variables.concat(merge_request.predefined_variables) end + + if external_pull_request_event? && external_pull_request + variables.concat(external_pull_request.predefined_variables) + end end end diff --git a/app/models/ci/pipeline_enums.rb b/app/models/ci/pipeline_enums.rb index 571c4271475..0c2bd0aa8eb 100644 --- a/app/models/ci/pipeline_enums.rb +++ b/app/models/ci/pipeline_enums.rb @@ -23,7 +23,8 @@ module Ci api: 5, external: 6, chat: 8, - merge_request_event: 10 + merge_request_event: 10, + external_pull_request_event: 11 } end diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb index fb74d96efe3..ec65482a846 100644 --- a/app/models/clusters/applications/jupyter.rb +++ b/app/models/clusters/applications/jupyter.rb @@ -85,7 +85,8 @@ module Clusters "clientId" => oauth_application.uid, "clientSecret" => oauth_application.secret, "callbackUrl" => callback_url, - "gitlabProjectIdWhitelist" => [project_id] + "gitlabProjectIdWhitelist" => cluster.projects.ids, + "gitlabGroupWhitelist" => cluster.groups.map(&:to_param) } }, "singleuser" => { @@ -101,10 +102,6 @@ module Clusters @crypto_key ||= SecureRandom.hex(32) end - def project_id - cluster&.project&.id - end - def gitlab_url Gitlab.config.gitlab.url end diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 444e1a82c97..a976093ac0c 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -10,15 +10,15 @@ module Clusters self.table_name = 'clusters' PROJECT_ONLY_APPLICATIONS = { - Applications::Jupyter.application_name => Applications::Jupyter, - Applications::Knative.application_name => Applications::Knative }.freeze APPLICATIONS = { Applications::Helm.application_name => Applications::Helm, Applications::Ingress.application_name => Applications::Ingress, Applications::CertManager.application_name => Applications::CertManager, + Applications::Prometheus.application_name => Applications::Prometheus, Applications::Runner.application_name => Applications::Runner, - Applications::Prometheus.application_name => Applications::Prometheus + Applications::Jupyter.application_name => Applications::Jupyter, + Applications::Knative.application_name => Applications::Knative }.merge(PROJECT_ONLY_APPLICATIONS).freeze DEFAULT_ENVIRONMENT = '*' KUBE_INGRESS_BASE_DOMAIN = 'KUBE_INGRESS_BASE_DOMAIN' diff --git a/app/models/external_pull_request.rb b/app/models/external_pull_request.rb new file mode 100644 index 00000000000..65ae8d95500 --- /dev/null +++ b/app/models/external_pull_request.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +# This model stores pull requests coming from external providers, such as +# GitHub, when GitLab project is set as CI/CD only and remote mirror. +# +# When setting up a remote mirror with GitHub we subscribe to push and +# pull_request webhook events. When a pull request is opened on GitHub, +# a webhook is sent out, we create or update the status of the pull +# request locally. +# +# When the mirror is updated and changes are pushed to branches we check +# if there are open pull requests for the source and target branch. +# If so, we create pipelines for external pull requests. +class ExternalPullRequest < ApplicationRecord + include Gitlab::Utils::StrongMemoize + include ShaAttribute + + belongs_to :project + + sha_attribute :source_sha + sha_attribute :target_sha + + validates :source_branch, presence: true + validates :target_branch, presence: true + validates :source_sha, presence: true + validates :target_sha, presence: true + validates :source_repository, presence: true + validates :target_repository, presence: true + validates :status, presence: true + + enum status: { + open: 1, + closed: 2 + } + + # We currently don't support pull requests from fork, so + # we are going to return an error to the webhook + validate :not_from_fork + + scope :by_source_branch, ->(branch) { where(source_branch: branch) } + scope :by_source_repository, -> (repository) { where(source_repository: repository) } + + def self.create_or_update_from_params(params) + find_params = params.slice(:project_id, :source_branch, :target_branch) + + safe_find_or_initialize_and_update(find: find_params, update: params) do |pull_request| + yield(pull_request) if block_given? + end + end + + def actual_branch_head? + actual_source_branch_sha == source_sha + end + + def from_fork? + source_repository != target_repository + end + + def source_ref + Gitlab::Git::BRANCH_REF_PREFIX + source_branch + end + + def predefined_variables + Gitlab::Ci::Variables::Collection.new.tap do |variables| + variables.append(key: 'CI_EXTERNAL_PULL_REQUEST_IID', value: pull_request_iid.to_s) + variables.append(key: 'CI_EXTERNAL_PULL_REQUEST_SOURCE_BRANCH_SHA', value: source_sha) + variables.append(key: 'CI_EXTERNAL_PULL_REQUEST_TARGET_BRANCH_SHA', value: target_sha) + variables.append(key: 'CI_EXTERNAL_PULL_REQUEST_SOURCE_BRANCH_NAME', value: source_branch) + variables.append(key: 'CI_EXTERNAL_PULL_REQUEST_TARGET_BRANCH_NAME', value: target_branch) + end + end + + private + + def actual_source_branch_sha + project.commit(source_ref)&.sha + end + + def not_from_fork + if from_fork? + errors.add(:base, 'Pull requests from fork are not supported') + end + end + + def self.safe_find_or_initialize_and_update(find:, update:) + safe_ensure_unique(retries: 1) do + model = find_or_initialize_by(find) + + if model.update(update) + yield(model) if block_given? + end + + model + end + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index bc98ae0ce28..95daa48d4bc 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1238,9 +1238,9 @@ class MergeRequest < ApplicationRecord compare_reports(Ci::CompareTestReportsService) end - def compare_reports(service_class) + def compare_reports(service_class, current_user = nil) with_reactive_cache(service_class.name) do |data| - unless service_class.new(project) + unless service_class.new(project, current_user) .latest?(base_pipeline, actual_head_pipeline, data) raise InvalidateReactiveCache end diff --git a/app/models/project.rb b/app/models/project.rb index 17b52d0578e..d948410e397 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -291,6 +291,8 @@ class Project < ApplicationRecord has_many :remote_mirrors, inverse_of: :project has_many :cycle_analytics_stages, class_name: 'Analytics::CycleAnalytics::ProjectStage' + has_many :external_pull_requests, inverse_of: :project + accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :project_feature, update_only: true accepts_nested_attributes_for :import_data diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index e2634692dc7..5c36b59f07b 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -294,6 +294,7 @@ class ProjectPolicy < BasePolicy enable :destroy_release enable :destroy_artifacts enable :daily_statistics + enable :admin_operations end rule { (mirror_available & can?(:admin_project)) | admin }.enable :admin_remote_mirror diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 8f8582afb43..4a7f62de9e1 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -18,7 +18,8 @@ module Ci Gitlab::Ci::Pipeline::Chain::Limit::Activity, Gitlab::Ci::Pipeline::Chain::Limit::JobActivity].freeze - def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, merge_request: nil, **options, &block) + # rubocop: disable Metrics/ParameterLists + def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, merge_request: nil, external_pull_request: nil, **options, &block) @pipeline = Ci::Pipeline.new command = Gitlab::Ci::Pipeline::Chain::Command.new( @@ -32,6 +33,7 @@ module Ci trigger_request: trigger_request, schedule: schedule, merge_request: merge_request, + external_pull_request: external_pull_request, ignore_skip_ci: ignore_skip_ci, save_incompleted: save_on_errors, seeds_block: block, @@ -62,6 +64,7 @@ module Ci pipeline end + # rubocop: enable Metrics/ParameterLists def execute!(*args, &block) execute(*args, &block).tap do |pipeline| diff --git a/app/services/external_pull_requests/create_pipeline_service.rb b/app/services/external_pull_requests/create_pipeline_service.rb new file mode 100644 index 00000000000..36411465ff1 --- /dev/null +++ b/app/services/external_pull_requests/create_pipeline_service.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# This service is responsible for creating a pipeline for a given +# ExternalPullRequest coming from other providers such as GitHub. + +module ExternalPullRequests + class CreatePipelineService < BaseService + def execute(pull_request) + return unless pull_request.open? && pull_request.actual_branch_head? + + create_pipeline_for(pull_request) + end + + private + + def create_pipeline_for(pull_request) + Ci::CreatePipelineService.new(project, current_user, create_params(pull_request)) + .execute(:external_pull_request_event, external_pull_request: pull_request) + end + + def create_params(pull_request) + { + ref: pull_request.source_ref, + source_sha: pull_request.source_sha, + target_sha: pull_request.target_sha + } + end + end +end diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb index 47c308c8280..35a4d2015fa 100644 --- a/app/services/git/base_hooks_service.rb +++ b/app/services/git/base_hooks_service.rb @@ -57,7 +57,9 @@ module Git Ci::CreatePipelineService .new(project, current_user, pipeline_params) - .execute(:push, pipeline_options) + .execute!(:push, pipeline_options) + rescue Ci::CreatePipelineService::CreateError => ex + log_pipeline_errors(ex) end def execute_project_hooks @@ -125,5 +127,29 @@ module Git project.mark_stuck_remote_mirrors_as_failed! project.update_remote_mirrors end + + def log_pipeline_errors(exception) + data = { + class: self.class.name, + correlation_id: Labkit::Correlation::CorrelationId.current_id.to_s, + project_id: project.id, + project_path: project.full_path, + message: "Error creating pipeline", + errors: exception.to_s, + pipeline_params: pipeline_params + } + + logger.warn(data) + end + + def logger + if Sidekiq.server? + Sidekiq.logger + else + # This service runs in Sidekiq, so this shouldn't ever be + # called, but this is included just in case. + Gitlab::ProjectServiceLogger + end + end end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 3555864f834..900e5063621 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -76,13 +76,16 @@ class IssuableBaseService < BaseService end def filter_labels - params[:add_label_ids] = labels_service.filter_labels_ids_in_param(:add_label_ids) if params[:add_label_ids] - params[:remove_label_ids] = labels_service.filter_labels_ids_in_param(:remove_label_ids) if params[:remove_label_ids] + label_ids_to_filter(:add_label_ids, :add_labels, false) + label_ids_to_filter(:remove_label_ids, :remove_labels, true) + label_ids_to_filter(:label_ids, :labels, false) + end - if params[:label_ids] - params[:label_ids] = labels_service.filter_labels_ids_in_param(:label_ids) - elsif params[:labels] - params[:label_ids] = labels_service.find_or_create_by_titles.map(&:id) + def label_ids_to_filter(label_id_key, label_key, find_only) + if params[label_id_key] + params[label_id_key] = labels_service.filter_labels_ids_in_param(label_id_key) + elsif params[label_key] + params[label_id_key] = labels_service.find_or_create_by_titles(label_key, find_only: find_only).map(&:id) end end diff --git a/app/services/labels/available_labels_service.rb b/app/services/labels/available_labels_service.rb index fe477d96970..8886e58d6ef 100644 --- a/app/services/labels/available_labels_service.rb +++ b/app/services/labels/available_labels_service.rb @@ -9,8 +9,8 @@ module Labels @params = params end - def find_or_create_by_titles - labels = params.delete(:labels) + def find_or_create_by_titles(key = :labels, find_only: false) + labels = params.delete(key) return [] unless labels @@ -23,7 +23,7 @@ module Labels include_ancestor_groups: true, title: label_name.strip, available_labels: available_labels - ).execute + ).execute(find_only: find_only) label end.compact diff --git a/app/services/labels/find_or_create_service.rb b/app/services/labels/find_or_create_service.rb index 628873519d7..a47dd42aea0 100644 --- a/app/services/labels/find_or_create_service.rb +++ b/app/services/labels/find_or_create_service.rb @@ -9,9 +9,9 @@ module Labels @params = params.dup.with_indifferent_access end - def execute(skip_authorization: false) + def execute(skip_authorization: false, find_only: false) @skip_authorization = skip_authorization - find_or_create_label + find_or_create_label(find_only: find_only) end private @@ -30,9 +30,11 @@ module Labels # Only creates the label if current_user can do so, if the label does not exist # and the user can not create the label, nil is returned # rubocop: disable CodeReuse/ActiveRecord - def find_or_create_label + def find_or_create_label(find_only: false) new_label = available_labels.find_by(title: title) + return new_label if find_only + if new_label.nil? && (skip_authorization || Ability.allowed?(current_user, :admin_label, parent)) create_params = params.except(:include_ancestor_groups) new_label = Labels::CreateService.new(create_params).execute(parent_type.to_sym => parent) diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index b28f80939ae..308a3a10d1a 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -18,6 +18,18 @@ module MergeRequests merge_request.target_project = find_target_project filter_params(merge_request) + + # merge_request.assign_attributes(...) below is a Rails + # method that only work if all the params it is passed have + # corresponding fields in the database. As there are no fields + # in the database for :add_label_ids and :remove_label_ids, we + # need to remove them from the params before the call to + # merge_request.assign_attributes(...) + # + # IssuableBaseService#process_label_ids takes care + # of the removal. + params[:label_ids] = process_label_ids(params, extra_label_ids: merge_request.label_ids.to_a) + merge_request.assign_attributes(params.to_h.compact) merge_request.compare_commits = [] diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb index b210004e6e1..0168b31005e 100644 --- a/app/services/merge_requests/push_options_handler_service.rb +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -100,7 +100,8 @@ module MergeRequests merge_request = ::MergeRequests::CreateService.new( project, current_user, - merge_request.attributes.merge(assignees: merge_request.assignees) + merge_request.attributes.merge(assignees: merge_request.assignees, + label_ids: merge_request.label_ids) ).execute end @@ -122,7 +123,9 @@ module MergeRequests title: push_options[:title], description: push_options[:description], target_branch: push_options[:target], - force_remove_source_branch: push_options[:remove_source_branch] + force_remove_source_branch: push_options[:remove_source_branch], + label: push_options[:label], + unlabel: push_options[:unlabel] } params.compact! @@ -134,6 +137,9 @@ module MergeRequests ) end + params[:add_labels] = params.delete(:label).keys if params.has_key?(:label) + params[:remove_labels] = params.delete(:unlabel).keys if params.has_key?(:unlabel) + params end diff --git a/app/views/projects/error_tracking/index.html.haml b/app/views/projects/error_tracking/index.html.haml index bc02c5f0e5a..96f61584a99 100644 --- a/app/views/projects/error_tracking/index.html.haml +++ b/app/views/projects/error_tracking/index.html.haml @@ -1,3 +1,3 @@ - page_title _('Errors') -#js-error_tracking{ data: error_tracking_data(@project) } +#js-error_tracking{ data: error_tracking_data(@current_user, @project) } diff --git a/app/views/projects/merge_requests/_how_to_merge.html.haml b/app/views/projects/merge_requests/_how_to_merge.html.haml index 15499c89ffb..928b54ea28f 100644 --- a/app/views/projects/merge_requests/_how_to_merge.html.haml +++ b/app/views/projects/merge_requests/_how_to_merge.html.haml @@ -13,12 +13,13 @@ %pre.dark#merge-info-1 - if @merge_request.for_fork? :preserve - git fetch #{h default_url_to_repo(@merge_request.source_project)} #{h @merge_request.source_branch} - git checkout -b #{h @merge_request.source_project_path}-#{h @merge_request.source_branch} FETCH_HEAD + -# All repo/branch refs have been quoted to allow support for special characters (such as #my-branch) + git fetch "#{h default_url_to_repo(@merge_request.source_project)}" "#{h @merge_request.source_branch}" + git checkout -b "#{h @merge_request.source_project_path}-#{h @merge_request.source_branch}" FETCH_HEAD - else :preserve git fetch origin - git checkout -b #{h @merge_request.source_branch} origin/#{h @merge_request.source_branch} + git checkout -b "#{h @merge_request.source_branch}" "origin/#{h @merge_request.source_branch}" %p %strong Step 2. Review the changes locally @@ -31,20 +32,20 @@ - if @merge_request.for_fork? :preserve git fetch origin - git checkout origin/#{h @merge_request.target_branch} - git merge --no-ff #{h @merge_request.source_project_path}-#{h @merge_request.source_branch} + git checkout "origin/#{h @merge_request.target_branch}" + git merge --no-ff "#{h @merge_request.source_project_path}-#{h @merge_request.source_branch}" - else :preserve git fetch origin - git checkout origin/#{h @merge_request.target_branch} - git merge --no-ff #{h @merge_request.source_branch} + git checkout "origin/#{h @merge_request.target_branch}" + git merge --no-ff "#{h @merge_request.source_branch}" %p %strong Step 4. Push the result of the merge to GitLab = clipboard_button(target: "pre#merge-info-4", title: "Copy commands to clipboard") %pre.dark#merge-info-4 :preserve - git push origin #{h @merge_request.target_branch} + git push origin "#{h @merge_request.target_branch}" - unless @merge_request.can_be_merged_by?(current_user) %p Note that pushing to GitLab requires write access to this repository. diff --git a/app/views/projects/mirrors/_instructions.html.haml b/app/views/projects/mirrors/_instructions.html.haml index 1a163cc4a54..7ff6c0a2019 100644 --- a/app/views/projects/mirrors/_instructions.html.haml +++ b/app/views/projects/mirrors/_instructions.html.haml @@ -3,6 +3,7 @@ %li = _('The repository must be accessible over <code>http://</code>, <code>https://</code>, <code>ssh://</code> or <code>git://</code>.').html_safe + %li= _('When using the <code>http://</code> or <code>https://</code> protocols, please provide the exact URL to the repository. HTTP redirects will not be followed.').html_safe %li= _('Include the username in the URL if required: <code>https://username@gitlab.company.com/group/project.git</code>.').html_safe %li - minutes = Gitlab.config.gitlab_shell.git_timeout / 60 diff --git a/app/views/shared/_import_form.html.haml b/app/views/shared/_import_form.html.haml index d0f9374e832..b2ea45d6f1a 100644 --- a/app/views/shared/_import_form.html.haml +++ b/app/views/shared/_import_form.html.haml @@ -27,6 +27,7 @@ %ul %li = _('The repository must be accessible over <code>http://</code>, <code>https://</code> or <code>git://</code>.').html_safe + %li= _('When using the <code>http://</code> or <code>https://</code> protocols, please provide the exact URL to the repository. HTTP redirects will not be followed.').html_safe %li = _('If your HTTP repository is not publicly accessible, add your credentials.') %li diff --git a/app/views/shared/members/_sort_dropdown.html.haml b/app/views/shared/members/_sort_dropdown.html.haml index feca109dade..5f3d49adff7 100644 --- a/app/views/shared/members/_sort_dropdown.html.haml +++ b/app/views/shared/members/_sort_dropdown.html.haml @@ -1,4 +1,4 @@ -= label_tag :sort_by, 'Sort by', class: 'col-form-label label-bold pr-2' += label_tag :sort_by, 'Sort by', class: 'col-form-label label-bold px-2' .dropdown.inline.qa-user-sort-dropdown = dropdown_toggle(member_sort_options_hash[@sort], { toggle: 'dropdown' }) %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 991a177018e..a33afd436b0 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -160,6 +160,7 @@ - repository_import - repository_remove_remote - system_hook_push +- update_external_pull_requests - update_merge_requests - update_project_statistics - upload_checksum diff --git a/app/workers/update_external_pull_requests_worker.rb b/app/workers/update_external_pull_requests_worker.rb new file mode 100644 index 00000000000..c5acfa82ada --- /dev/null +++ b/app/workers/update_external_pull_requests_worker.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class UpdateExternalPullRequestsWorker + include ApplicationWorker + + def perform(project_id, user_id, ref) + project = Project.find_by_id(project_id) + return unless project + + user = User.find_by_id(user_id) + return unless user + + branch = Gitlab::Git.branch_name(ref) + return unless branch + + external_pull_requests = project.external_pull_requests + .by_source_repository(project.import_source) + .by_source_branch(branch) + + external_pull_requests.find_each do |pull_request| + ExternalPullRequests::CreatePipelineService.new(project, user) + .execute(pull_request) + end + end +end diff --git a/changelogs/unreleased/59729-estimate-quick-action-does-not-produce-correct-time-for-1mo.yml b/changelogs/unreleased/59729-estimate-quick-action-does-not-produce-correct-time-for-1mo.yml new file mode 100644 index 00000000000..1962a662179 --- /dev/null +++ b/changelogs/unreleased/59729-estimate-quick-action-does-not-produce-correct-time-for-1mo.yml @@ -0,0 +1,5 @@ +--- +title: Fix parsing of months in time tracking commands +merge_request: 32165 +author: +type: fixed diff --git a/changelogs/unreleased/66467-enable-error-tracking-only-user-can-read-sentry-logs.yml b/changelogs/unreleased/66467-enable-error-tracking-only-user-can-read-sentry-logs.yml new file mode 100644 index 00000000000..b152943942f --- /dev/null +++ b/changelogs/unreleased/66467-enable-error-tracking-only-user-can-read-sentry-logs.yml @@ -0,0 +1,5 @@ +--- +title: Display `more information` docs link on error tracking page when users do not have permissions to enable that feature +merge_request: 32365 +author: Romain Maneschi +type: fixed diff --git a/changelogs/unreleased/66616-follow-up-documentation-for-merge-trains-cancel-when-running.yml b/changelogs/unreleased/66616-follow-up-documentation-for-merge-trains-cancel-when-running.yml new file mode 100644 index 00000000000..54bec43815c --- /dev/null +++ b/changelogs/unreleased/66616-follow-up-documentation-for-merge-trains-cancel-when-running.yml @@ -0,0 +1,5 @@ +--- +title: Update merge train documentation +merge_request: 32218 +author: +type: changed diff --git a/changelogs/unreleased/67037-user-content-gitlab-static-net-brings-back-404-only.yml b/changelogs/unreleased/67037-user-content-gitlab-static-net-brings-back-404-only.yml new file mode 100644 index 00000000000..8d4f816af2e --- /dev/null +++ b/changelogs/unreleased/67037-user-content-gitlab-static-net-brings-back-404-only.yml @@ -0,0 +1,5 @@ +--- +title: Default the asset proxy whitelist to the installation domain +merge_request: 32703 +author: +type: fixed diff --git a/changelogs/unreleased/add-label-push-opts.yml b/changelogs/unreleased/add-label-push-opts.yml new file mode 100644 index 00000000000..1289020e4e5 --- /dev/null +++ b/changelogs/unreleased/add-label-push-opts.yml @@ -0,0 +1,5 @@ +--- +title: Support adding and removing labels w/ push opts +merge_request: 31831 +author: +type: added diff --git a/changelogs/unreleased/group_level_jupyterhub.yml b/changelogs/unreleased/group_level_jupyterhub.yml new file mode 100644 index 00000000000..81fc7600e0e --- /dev/null +++ b/changelogs/unreleased/group_level_jupyterhub.yml @@ -0,0 +1,5 @@ +--- +title: Group level JupyterHub +merge_request: 32512 +author: +type: added diff --git a/changelogs/unreleased/instance-group-level-knative.yml b/changelogs/unreleased/instance-group-level-knative.yml new file mode 100644 index 00000000000..5108334a3ea --- /dev/null +++ b/changelogs/unreleased/instance-group-level-knative.yml @@ -0,0 +1,5 @@ +--- +title: Allow Knative to be installed on group and instance level clusters +merge_request: 32128 +author: +type: added diff --git a/changelogs/unreleased/quote-branch-names-in-instructions.yml b/changelogs/unreleased/quote-branch-names-in-instructions.yml new file mode 100644 index 00000000000..fe5964c8918 --- /dev/null +++ b/changelogs/unreleased/quote-branch-names-in-instructions.yml @@ -0,0 +1,5 @@ +--- +title: Quote branch names in how to merge instructions +merge_request: 32639 +author: Lee Tickett +type: fixed diff --git a/changelogs/unreleased/sh-add-margin-member-list.yml b/changelogs/unreleased/sh-add-margin-member-list.yml new file mode 100644 index 00000000000..35d35aad8bf --- /dev/null +++ b/changelogs/unreleased/sh-add-margin-member-list.yml @@ -0,0 +1,5 @@ +--- +title: Add padding to left of "Sort by" in members dropdown +merge_request: 32602 +author: +type: other diff --git a/changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml b/changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml new file mode 100644 index 00000000000..b334355cab6 --- /dev/null +++ b/changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml @@ -0,0 +1,5 @@ +--- +title: Log errors for failed pipeline creation in PostReceive +merge_request: 32633 +author: +type: other diff --git a/changelogs/unreleased/use_default_external_auth_label_empty.yml b/changelogs/unreleased/use_default_external_auth_label_empty.yml new file mode 100644 index 00000000000..9c1039b0875 --- /dev/null +++ b/changelogs/unreleased/use_default_external_auth_label_empty.yml @@ -0,0 +1,6 @@ +--- +title: Prevent empty external authorization classification labels from overriding + the default label +merge_request: 32517 +author: Will Chandler +type: fixed diff --git a/config/initializers/chronic_duration.rb b/config/initializers/chronic_duration.rb index b65b06c813a..aa43eef434c 100644 --- a/config/initializers/chronic_duration.rb +++ b/config/initializers/chronic_duration.rb @@ -1 +1,5 @@ +# frozen_string_literal: true + ChronicDuration.raise_exceptions = true + +ChronicDuration.prepend Gitlab::Patch::ChronicDuration diff --git a/config/routes.rb b/config/routes.rb index 02a405a91f8..a622ce268da 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -107,6 +107,7 @@ Rails.application.routes.draw do draw :instance_statistics Gitlab.ee do + draw :security draw :smartcard draw :jira_connect draw :username diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 7edec576f9a..e89e9657314 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -115,3 +115,4 @@ - [export_csv, 1] - [incident_management, 2] - [jira_connect, 1] + - [update_external_pull_requests, 3] diff --git a/danger/database/Dangerfile b/danger/database/Dangerfile index 3550cb7eabf..5cdad09db6e 100644 --- a/danger/database/Dangerfile +++ b/danger/database/Dangerfile @@ -1,7 +1,13 @@ # frozen_string_literal: true -SCHEMA_NOT_UPDATED_MESSAGE = <<~MSG -**New %<migrations>s added but %<schema>s wasn't updated.** +gitlab_danger = GitlabDanger.new(helper.gitlab_helper) + +SCHEMA_NOT_UPDATED_MESSAGE_SHORT = <<~MSG +New %<migrations>s added but %<schema>s wasn't updated. +MSG + +SCHEMA_NOT_UPDATED_MESSAGE_FULL = <<~MSG +**#{SCHEMA_NOT_UPDATED_MESSAGE_SHORT}** Usually, when adding new %<migrations>s, %<schema>s should be updated too (unless the migration isn't changing the DB schema @@ -29,14 +35,18 @@ geo_db_schema_updated = !git.modified_files.grep(%r{\Aee/db/geo/schema\.rb}).emp non_geo_migration_created = !git.added_files.grep(%r{\A(db/(post_)?migrate)/}).empty? geo_migration_created = !git.added_files.grep(%r{\Aee/db/geo/(post_)?migrate/}).empty? +format_str = gitlab_danger.ci? ? SCHEMA_NOT_UPDATED_MESSAGE_FULL : SCHEMA_NOT_UPDATED_MESSAGE_SHORT + if non_geo_migration_created && !non_geo_db_schema_updated - warn format(SCHEMA_NOT_UPDATED_MESSAGE, migrations: 'migrations', schema: gitlab.html_link("db/schema.rb")) + warn format(format_str, migrations: 'migrations', schema: gitlab_danger.html_link("db/schema.rb")) end if geo_migration_created && !geo_db_schema_updated - warn format(SCHEMA_NOT_UPDATED_MESSAGE, migrations: 'Geo migrations', schema: gitlab.html_link("ee/db/geo/schema.rb")) + warn format(format_str, migrations: 'Geo migrations', schema: gitlab_danger.html_link("ee/db/geo/schema.rb")) end +return unless gitlab_danger.ci? + db_paths_to_review = helper.changes_by_category[:database] if gitlab.mr_labels.include?('database') || db_paths_to_review.any? diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile index 96c0d9b7478..ad64c3a6c60 100644 --- a/danger/documentation/Dangerfile +++ b/danger/documentation/Dangerfile @@ -6,20 +6,22 @@ unless docs_paths_to_review.empty? message 'This merge request adds or changes files that require a review ' \ 'from the Technical Writing team.' - markdown(<<~MARKDOWN) -## Documentation review + if GitlabDanger.new(helper.gitlab_helper).ci? + markdown(<<~MARKDOWN) + ## Documentation review -The following files require a review from a technical writer: + The following files require a review from a technical writer: -* #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")} + * #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")} -The review does not need to block merging this merge request. See the: + The review does not need to block merging this merge request. See the: -- [DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages) for the appropriate technical writer for this review. -- [Documentation workflows](https://docs.gitlab.com/ee/development/documentation/workflow.html) for information on when to assign a merge request for review. - MARKDOWN + - [DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages) for the appropriate technical writer for this review. + - [Documentation workflows](https://docs.gitlab.com/ee/development/documentation/workflow.html) for information on when to assign a merge request for review. + MARKDOWN - unless gitlab.mr_labels.include?('Documentation') - warn 'This merge request is missing the ~Documentation label.' + unless gitlab.mr_labels.include?('Documentation') + warn 'This merge request is missing the ~Documentation label.' + end end end diff --git a/danger/duplicate_yarn_dependencies/Dangerfile b/danger/duplicate_yarn_dependencies/Dangerfile index 25f81ec86a4..492b888d00e 100644 --- a/danger/duplicate_yarn_dependencies/Dangerfile +++ b/danger/duplicate_yarn_dependencies/Dangerfile @@ -1,6 +1,6 @@ # frozen_string_literal: true -return unless helper.all_changed_files.include? 'yarn.lock' +return unless helper.all_changed_files.include?('yarn.lock') duplicate = `node_modules/.bin/yarn-deduplicate --list --strategy fewer yarn.lock` .split(/$/) @@ -11,17 +11,19 @@ return if duplicate.empty? warn 'This merge request has introduced duplicated yarn dependencies.' -markdown(<<~MARKDOWN) - ## Duplicate yarn dependencies +if GitlabDanger.new(helper.gitlab_helper).ci? + markdown(<<~MARKDOWN) + ## Duplicate yarn dependencies - The following dependencies should be de-duplicated: + The following dependencies should be de-duplicated: - * #{duplicate.map { |path| "`#{path}`" }.join("\n* ")} + * #{duplicate.map { |path| "`#{path}`" }.join("\n* ")} - Please run the following command and commit the changes to `yarn.lock`: + Please run the following command and commit the changes to `yarn.lock`: - ``` - node_modules/.bin/yarn-deduplicate --strategy fewer yarn.lock \\ - && yarn install - ``` -MARKDOWN + ``` + node_modules/.bin/yarn-deduplicate --strategy fewer yarn.lock \\ + && yarn install + ``` + MARKDOWN +end diff --git a/danger/eslint/Dangerfile b/danger/eslint/Dangerfile index 4916cacfd7e..92830bd7706 100644 --- a/danger/eslint/Dangerfile +++ b/danger/eslint/Dangerfile @@ -13,17 +13,19 @@ return if eslint_candidates.empty? warn 'This merge request changed files with disabled eslint rules. Please consider fixing them.' -markdown(<<~MARKDOWN) - ## Disabled eslint rules +if GitlabDanger.new(helper.gitlab_helper).ci? + markdown(<<~MARKDOWN) + ## Disabled eslint rules - The following files have disabled `eslint` rules. Please consider fixing them: + The following files have disabled `eslint` rules. Please consider fixing them: - * #{eslint_candidates.map { |path| "`#{path}`" }.join("\n* ")} + * #{eslint_candidates.map { |path| "`#{path}`" }.join("\n* ")} - Run the following command for more details + Run the following command for more details - ``` - node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \\ - #{eslint_candidates.map { |path| " '#{path}'" }.join(" \\\n")} - ``` -MARKDOWN + ``` + node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \\ + #{eslint_candidates.map { |path| " '#{path}'" }.join(" \\\n")} + ``` + MARKDOWN +end diff --git a/danger/frozen_string/Dangerfile b/danger/frozen_string/Dangerfile index b9687ef6b83..8d3ac3dee68 100644 --- a/danger/frozen_string/Dangerfile +++ b/danger/frozen_string/Dangerfile @@ -16,11 +16,13 @@ if files_to_fix.any? warn 'This merge request adds files that do not enforce frozen string literal. ' \ 'See https://gitlab.com/gitlab-org/gitlab-ce/issues/47424 for more information.' - markdown(<<~MARKDOWN) - ## Enable Frozen String Literal + if GitlabDanger.new(helper.gitlab_helper).ci? + markdown(<<~MARKDOWN) + ## Enable Frozen String Literal - The following files should have `#{MAGIC_COMMENT}` on the first line: + The following files should have `#{MAGIC_COMMENT}` on the first line: - * #{files_to_fix.map { |path| "`#{path}`" }.join("\n* ")} - MARKDOWN + * #{files_to_fix.map { |path| "`#{path}`" }.join("\n* ")} + MARKDOWN + end end diff --git a/danger/gemfile/Dangerfile b/danger/gemfile/Dangerfile index dfe64f79d7b..07c4c07cfe8 100644 --- a/danger/gemfile/Dangerfile +++ b/danger/gemfile/Dangerfile @@ -1,5 +1,9 @@ -GEMFILE_LOCK_NOT_UPDATED_MESSAGE = <<~MSG.freeze -**%<gemfile>s was updated but %<gemfile_lock>s wasn't updated.** +GEMFILE_LOCK_NOT_UPDATED_MESSAGE_SHORT = <<~MSG.freeze +%<gemfile>s was updated but %<gemfile_lock>s wasn't updated. +MSG + +GEMFILE_LOCK_NOT_UPDATED_MESSAGE_FULL = <<~MSG.freeze +**#{GEMFILE_LOCK_NOT_UPDATED_MESSAGE_SHORT}** Usually, when %<gemfile>s is updated, you should run ``` @@ -19,5 +23,14 @@ gemfile_modified = git.modified_files.include?("Gemfile") gemfile_lock_modified = git.modified_files.include?("Gemfile.lock") if gemfile_modified && !gemfile_lock_modified - warn format(GEMFILE_LOCK_NOT_UPDATED_MESSAGE, gemfile: gitlab.html_link("Gemfile"), gemfile_lock: gitlab.html_link("Gemfile.lock")) + gitlab_danger = GitlabDanger.new(helper.gitlab_helper) + + format_str = gitlab_danger.ci? ? GEMFILE_LOCK_NOT_UPDATED_MESSAGE_FULL : GEMFILE_LOCK_NOT_UPDATED_MESSAGE_SHORT + + message = format(format_str, + gemfile: gitlab_danger.html_link("Gemfile"), + gemfile_lock: gitlab_danger.html_link("Gemfile.lock") + ) + + warn(message) end diff --git a/danger/prettier/Dangerfile b/danger/prettier/Dangerfile index 37c4b78a213..0be75db8baa 100644 --- a/danger/prettier/Dangerfile +++ b/danger/prettier/Dangerfile @@ -19,21 +19,23 @@ return if unpretty.empty? warn 'This merge request changed frontend files without pretty printing them.' -markdown(<<~MARKDOWN) - ## Pretty print Frontend files +if GitlabDanger.new(helper.gitlab_helper).ci? + markdown(<<~MARKDOWN) + ## Pretty print Frontend files - The following files should have been pretty printed with `prettier`: + The following files should have been pretty printed with `prettier`: - * #{unpretty.map { |path| "`#{path}`" }.join("\n* ")} + * #{unpretty.map { |path| "`#{path}`" }.join("\n* ")} - Please run + Please run - ``` - node_modules/.bin/prettier --write \\ - #{unpretty.map { |path| " '#{path}'" }.join(" \\\n")} - ``` + ``` + node_modules/.bin/prettier --write \\ + #{unpretty.map { |path| " '#{path}'" }.join(" \\\n")} + ``` - Also consider auto-formatting [on-save]. + Also consider auto-formatting [on-save]. - [on-save]: https://docs.gitlab.com/ee/development/new_fe_guide/style/prettier.html -MARKDOWN + [on-save]: https://docs.gitlab.com/ee/development/new_fe_guide/style/prettier.html + MARKDOWN +end diff --git a/db/migrate/20190829131130_create_external_pull_requests.rb b/db/migrate/20190829131130_create_external_pull_requests.rb new file mode 100644 index 00000000000..0c3168807ec --- /dev/null +++ b/db/migrate/20190829131130_create_external_pull_requests.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class CreateExternalPullRequests < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX = 'index_external_pull_requests_on_project_and_branches' + + def change + create_table :external_pull_requests do |t| + t.timestamps_with_timezone null: false + t.references :project, null: false, foreign_key: { on_delete: :cascade }, index: false + t.integer :pull_request_iid, null: false + t.integer :status, null: false, limit: 2 + t.string :source_branch, null: false, limit: 255 + t.string :target_branch, null: false, limit: 255 + t.string :source_repository, null: false, limit: 255 + t.string :target_repository, null: false, limit: 255 + t.binary :source_sha, null: false + t.binary :target_sha, null: false + + t.index [:project_id, :source_branch, :target_branch], unique: true, name: INDEX + end + end +end diff --git a/db/migrate/20190830075508_add_external_pull_request_id_to_ci_pipelines.rb b/db/migrate/20190830075508_add_external_pull_request_id_to_ci_pipelines.rb new file mode 100644 index 00000000000..5abf56742b1 --- /dev/null +++ b/db/migrate/20190830075508_add_external_pull_request_id_to_ci_pipelines.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddExternalPullRequestIdToCiPipelines < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + add_column :ci_pipelines, :external_pull_request_id, :bigint + end + + def down + remove_column :ci_pipelines, :external_pull_request_id + end +end diff --git a/db/migrate/20190830080123_add_index_to_ci_pipelines_external_pull_request.rb b/db/migrate/20190830080123_add_index_to_ci_pipelines_external_pull_request.rb new file mode 100644 index 00000000000..d2f5ad7a420 --- /dev/null +++ b/db/migrate/20190830080123_add_index_to_ci_pipelines_external_pull_request.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexToCiPipelinesExternalPullRequest < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_pipelines, :external_pull_request_id, where: 'external_pull_request_id IS NOT NULL' + end + + def down + remove_concurrent_index :ci_pipelines, :external_pull_request_id + end +end diff --git a/db/migrate/20190830080626_add_foreign_key_to_ci_pipelines_external_pull_request.rb b/db/migrate/20190830080626_add_foreign_key_to_ci_pipelines_external_pull_request.rb new file mode 100644 index 00000000000..b38fda83047 --- /dev/null +++ b/db/migrate/20190830080626_add_foreign_key_to_ci_pipelines_external_pull_request.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddForeignKeyToCiPipelinesExternalPullRequest < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :ci_pipelines, :external_pull_requests, column: :external_pull_request_id, on_delete: :nullify + end + + def down + remove_foreign_key :ci_pipelines, :external_pull_requests + end +end diff --git a/db/schema.rb b/db/schema.rb index 61f7787f192..6ddfb8bcb39 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -754,7 +754,9 @@ ActiveRecord::Schema.define(version: 2019_09_05_223900) do t.integer "merge_request_id" t.binary "source_sha" t.binary "target_sha" + t.bigint "external_pull_request_id" t.index ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id" + t.index ["external_pull_request_id"], name: "index_ci_pipelines_on_external_pull_request_id", where: "(external_pull_request_id IS NOT NULL)" t.index ["merge_request_id"], name: "index_ci_pipelines_on_merge_request_id", where: "(merge_request_id IS NOT NULL)" t.index ["pipeline_schedule_id"], name: "index_ci_pipelines_on_pipeline_schedule_id" t.index ["project_id", "iid"], name: "index_ci_pipelines_on_project_id_and_iid", unique: true, where: "(iid IS NOT NULL)" @@ -1323,6 +1325,21 @@ ActiveRecord::Schema.define(version: 2019_09_05_223900) do t.index ["target_type", "target_id"], name: "index_events_on_target_type_and_target_id" end + create_table "external_pull_requests", force: :cascade do |t| + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.bigint "project_id", null: false + t.integer "pull_request_iid", null: false + t.integer "status", limit: 2, null: false + t.string "source_branch", limit: 255, null: false + t.string "target_branch", limit: 255, null: false + t.string "source_repository", limit: 255, null: false + t.string "target_repository", limit: 255, null: false + t.binary "source_sha", null: false + t.binary "target_sha", null: false + t.index ["project_id", "source_branch", "target_branch"], name: "index_external_pull_requests_on_project_and_branches", unique: true + end + create_table "feature_gates", id: :serial, force: :cascade do |t| t.string "feature_key", null: false t.string "key", null: false @@ -3785,6 +3802,7 @@ ActiveRecord::Schema.define(version: 2019_09_05_223900) do add_foreign_key "ci_pipeline_variables", "ci_pipelines", column: "pipeline_id", name: "fk_f29c5f4380", on_delete: :cascade add_foreign_key "ci_pipelines", "ci_pipeline_schedules", column: "pipeline_schedule_id", name: "fk_3d34ab2e06", on_delete: :nullify add_foreign_key "ci_pipelines", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_262d4c2d19", on_delete: :nullify + add_foreign_key "ci_pipelines", "external_pull_requests", name: "fk_190998ef09", on_delete: :nullify add_foreign_key "ci_pipelines", "merge_requests", name: "fk_a23be95014", on_delete: :cascade add_foreign_key "ci_pipelines", "projects", name: "fk_86635dbd80", on_delete: :cascade add_foreign_key "ci_runner_namespaces", "ci_runners", column: "runner_id", on_delete: :cascade @@ -3849,6 +3867,7 @@ ActiveRecord::Schema.define(version: 2019_09_05_223900) do add_foreign_key "events", "namespaces", column: "group_id", on_delete: :cascade add_foreign_key "events", "projects", on_delete: :cascade add_foreign_key "events", "users", column: "author_id", name: "fk_edfd187b6f", on_delete: :cascade + add_foreign_key "external_pull_requests", "projects", on_delete: :cascade add_foreign_key "fork_network_members", "fork_networks", on_delete: :cascade add_foreign_key "fork_network_members", "projects", column: "forked_from_project_id", name: "fk_b01280dae4", on_delete: :nullify add_foreign_key "fork_network_members", "projects", on_delete: :cascade diff --git a/doc/administration/database_load_balancing.md b/doc/administration/database_load_balancing.md index 64eca0b00f6..f643d853d10 100644 --- a/doc/administration/database_load_balancing.md +++ b/doc/administration/database_load_balancing.md @@ -146,7 +146,11 @@ The following options can be set: | `use_tcp` | Lookup DNS resources using TCP instead of UDP | false | If `record_type` is set to `SRV`, GitLab will continue to use a round-robin algorithm -and will ignore the `weight` and `priority` in the record. +and will ignore the `weight` and `priority` in the record. Since SRV records usually +return hostnames instead of IPs, GitLab will look for the IPs of returned hostnames +in the additional section of the SRV response. If no IP is found for a hostname, Gitlab +will query the configured `nameserver` for ANY record for each such hostname looking for A or AAAA records, eventually +dropping this hostname from rotation if it can't resolve its IP. The `interval` value specifies the _minimum_ time between checks. If the A record has a TTL greater than this value, then service discovery will honor said @@ -212,28 +216,25 @@ without it immediately leading to errors being presented to the users. ## Logging -The load balancer logs various messages, such as: +The load balancer logs various events in +[`database_load_balancing.log`](logs.md#database_load_balancinglog-premium-only), such as - When a host is marked as offline - When a host comes back online - When all secondaries are offline +- When a read is retried on a different host due to a query conflict -Each log message contains the tag `[DB-LB]` to make searching/filtering of such -log entries easier. For example: +The log is structured with each entry a JSON object containing at least: -``` -[DB-LB] Host 10.123.2.5 came back online -[DB-LB] Marking host 10.123.2.7 as offline -[DB-LB] Marking host 10.123.2.7 as offline -[DB-LB] Marking host 10.123.2.7 as offline -[DB-LB] Marking host 10.123.2.7 as offline -[DB-LB] Marking host 10.123.2.7 as offline -[DB-LB] Host 10.123.2.6 came back online -[DB-LB] Marking host 10.123.2.7 as offline -[DB-LB] Marking host 10.123.2.7 as offline -[DB-LB] Marking host 10.123.2.7 as offline -[DB-LB] Host 10.123.2.7 came back online -[DB-LB] Host 10.123.2.7 came back online +- An `event` field useful for filtering. +- A human-readable `message` field. +- Some event-specific metadata. For example, `db_host` +- Contextual information that is always logged. For example, `severity` and `time`. + +For example: + +```json +{"severity":"INFO","time":"2019-09-02T12:12:01.728Z","correlation_id":"abcdefg","event":"host_online","message":"Host came back online","db_host":"111.222.333.444","db_port":null,"tag":"rails.database_load_balancing","environment":"production","hostname":"web-example-1","fqdn":"gitlab.example.com","path":null,"params":null} ``` ## Handling Stale Reads diff --git a/doc/administration/logs.md b/doc/administration/logs.md index 4c43a434817..c51b53c596e 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -339,11 +339,11 @@ installations from source. [Rack Attack]: ../security/rack_attack.md [Rate Limit]: ../user/admin_area/settings/rate_limits_on_raw_endpoints.md -## `database_load_balancing.log` +## `database_load_balancing.log` **(PREMIUM ONLY)** > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/15442) in GitLab 12.3. -This log is used for observability of [Database Load Balancing](database_load_balancing.md). +Contains details of GitLab's [Database Load Balancing](database_load_balancing.md). It is stored at: - `/var/log/gitlab/gitlab-rails/database_load_balancing.log` for Omnibus GitLab packages. diff --git a/doc/api/merge_request_approvals.md b/doc/api/merge_request_approvals.md index b73fe38f53e..d19f11ba1d4 100644 --- a/doc/api/merge_request_approvals.md +++ b/doc/api/merge_request_approvals.md @@ -525,6 +525,77 @@ PUT /projects/:id/merge_requests/:merge_request_iid/approvers } ``` +### Get the approval state of merge requests + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/13712) in [GitLab Starter](https://about.gitlab.com/pricing/) 12.3. + +You can request information about a merge request's approval state by using the following endpoint: + +``` +GET /projects/:id/merge_requests/:merge_request_iid/approval_state +``` + +The `approval_rules_overwritten` will be `true` if the merge request level rules +are created for the merge request. If there's none, it'll be `false`. + +This includes additional information about the users who have already approved +(`approved_by`) and whether a rule is already approved (`approved`). + +**Parameters:** + +| Attribute | Type | Required | Description | +|----------------------|---------|----------|---------------------| +| `id` | integer | yes | The ID of a project | +| `merge_request_iid` | integer | yes | The IID of MR | + +```json +{ + "approval_rules_overwritten": true, + "rules": [ + { + "id": 1, + "name": "Ruby", + "rule_type": "regular", + "eligible_approvers": [ + { + "id": 4, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + } + ], + "approvals_required": 2, + "users": [ + { + "id": 4, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + } + ], + "groups": [], + "contains_hidden_groups": false, + "approved_by": [ + { + "id": 4, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + } + ], + "source_rule": null, + "approved": true + } + ] +} +``` + ### Get merge request level rules >**Note:** This API endpoint is only available on 12.3 Starter and above. diff --git a/doc/ci/examples/README.md b/doc/ci/examples/README.md index 00995f881da..f9612d0c53d 100644 --- a/doc/ci/examples/README.md +++ b/doc/ci/examples/README.md @@ -22,7 +22,7 @@ The following table lists examples with step-by-step tutorials that are containe | Use case | Resource | |:----------------------------|:---------------------------------------------------------------------------------------------------------------------------| -| Browser performance testing | [Browser Performance Testing with the Sitespeed.io container](browser_performance.md). | +| Browser performance testing | [Browser Performance Testing with the Sitespeed.io container](../../user/project/merge_requests/browser_performance_testing.md). | | Clojure | [Test a Clojure application with GitLab CI/CD](test-clojure-application.md). | | Deployment with Dpl | [Using `dpl` as deployment tool](deployment/README.md). | | Elixir | [Testing a Phoenix application with GitLab CI/CD](test_phoenix_app_with_gitlab_ci_cd/index.md). | diff --git a/doc/ci/examples/browser_performance.md b/doc/ci/examples/browser_performance.md index 3266e5dc62e..4a73fe2e62c 100644 --- a/doc/ci/examples/browser_performance.md +++ b/doc/ci/examples/browser_performance.md @@ -1,158 +1,5 @@ --- -type: howto +redirect_to: '../../user/project/merge_requests/browser_performance_testing.md#configuring-browser-performance-testing' --- -# Browser Performance Testing with the sitespeed.io container - -NOTE: **Note:** -The job definition shown below is supported on GitLab 11.5 and later versions. -It also requires the GitLab Runner 11.5 or later. -For earlier versions, use the [previous job definitions](#previous-job-definitions). - -This example shows how to run the -[sitespeed.io container](https://hub.docker.com/r/sitespeedio/sitespeed.io/) on -your code by using GitLab CI/CD and [sitespeed.io](https://www.sitespeed.io) -using Docker-in-Docker. - -First, you need GitLab Runner with -[docker-in-docker build](../docker/using_docker_build.md#use-docker-in-docker-workflow-with-docker-executor). - -Once you set up the Runner, add a new job to `.gitlab-ci.yml` that -generates the expected report: - -```yaml -performance: - stage: performance - image: docker:git - variables: - URL: https://example.com - services: - - docker:stable-dind - script: - - mkdir gitlab-exporter - - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js - - mkdir sitespeed-results - - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:6.3.1 --plugins.add ./gitlab-exporter --outputFolder sitespeed-results $URL - - mv sitespeed-results/data/performance.json performance.json - artifacts: - paths: - - sitespeed-results/ - reports: - performance: performance.json -``` - -The above example will create a `performance` job in your CI/CD pipeline and will run -sitespeed.io against the webpage you defined in `URL` to gather key metrics. -The [GitLab plugin](https://gitlab.com/gitlab-org/gl-performance) for -sitespeed.io is downloaded in order to save the report as a -[Performance report artifact](../yaml/README.md#artifactsreportsperformance-premium) -that you can later download and analyze. -Due to implementation limitations we always take the latest Performance artifact available. - -The full HTML sitespeed.io report will also be saved as an artifact, and if you have -[GitLab Pages](../../user/project/pages/index.md) enabled, it can be viewed -directly in your browser. - -For further customization options for sitespeed.io, including the ability to -provide a list of URLs to test, please see the -[Sitespeed.io Configuration](https://www.sitespeed.io/documentation/sitespeed.io/configuration/) documentation. - -TIP: **Tip:** -For [GitLab Premium](https://about.gitlab.com/pricing/) users, key metrics are automatically -extracted and shown right in the merge request widget. -[Learn more on Browser Performance Testing in merge requests](../../user/project/merge_requests/browser_performance_testing.md). - -## Performance testing on Review Apps - -The above CI YML is great for testing against static environments, and it can -be extended for dynamic environments. There are a few extra steps to take to -set this up: - -1. The `performance` job should run after the dynamic environment has started. -1. In the `review` job, persist the hostname and upload it as an artifact so - it's available to the `performance` job (the same can be done for static - environments like staging and production to unify the code path). Saving it - as an artifact is as simple as `echo $CI_ENVIRONMENT_URL > environment_url.txt` - in your job's `script`. -1. In the `performance` job, read the previous artifact into an environment - variable, like `$CI_ENVIRONMENT_URL`, and use it to parameterize the test - URLs. -1. You can now run the sitespeed.io container against the desired hostname and - paths. - -Your `.gitlab-ci.yml` file would look like: - -```yaml -stages: - - deploy - - performance - -review: - stage: deploy - environment: - name: review/$CI_COMMIT_REF_SLUG - url: http://$CI_COMMIT_REF_SLUG.$APPS_DOMAIN - script: - - run_deploy_script - - echo $CI_ENVIRONMENT_URL > environment_url.txt - artifacts: - paths: - - environment_url.txt - only: - - branches - except: - - master - -performance: - stage: performance - image: docker:git - services: - - docker:stable-dind - dependencies: - - review - script: - - export CI_ENVIRONMENT_URL=$(cat environment_url.txt) - - mkdir gitlab-exporter - - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js - - mkdir sitespeed-results - - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:6.3.1 --plugins.add ./gitlab-exporter --outputFolder sitespeed-results "$CI_ENVIRONMENT_URL" - - mv sitespeed-results/data/performance.json performance.json - artifacts: - paths: - - sitespeed-results/ - reports: - performance: performance.json -``` - -A complete example can be found in our [Auto DevOps CI YML](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml). - -## Previous job definitions - -CAUTION: **Caution:** -Before GitLab 11.5, Performance job and artifact had to be named specifically -to automatically extract report data and show it in the merge request widget. -While these old job definitions are still maintained they have been deprecated -and may be removed in next major release, GitLab 12.0. -You are advised to update your current `.gitlab-ci.yml` configuration to reflect that change. - -For GitLab 11.4 and earlier, the job should look like: - -```yaml -performance: - stage: performance - image: docker:git - variables: - URL: https://example.com - services: - - docker:stable-dind - script: - - mkdir gitlab-exporter - - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js - - mkdir sitespeed-results - - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:6.3.1 --plugins.add ./gitlab-exporter --outputFolder sitespeed-results $URL - - mv sitespeed-results/data/performance.json performance.json - artifacts: - paths: - - performance.json - - sitespeed-results/ -``` +This document was moved to [another location](../../user/project/merge_requests/browser_performance_testing.md#configuring-browser-performance-testing). diff --git a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md index 126e12e460f..d68fba82f4b 100644 --- a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md +++ b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md @@ -61,6 +61,20 @@ CAUTION: **Warning:** Make sure your `gitlab-ci.yml` file is [configured properly for pipelines for merge requests](../index.md#configuring-pipelines-for-merge-requests), otherwise pipelines for merged results won't run and your merge requests will be stuck in an unresolved state. +## Automatic pipeline cancelation + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/12996) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.3. + +GitLab CI can detect the presence of redundant pipelines, +and will cancel them automatically in order to conserve CI resources. + +When a user merges a merge request immediately within an ongoing merge +train, the train will be reconstructed, as it will recreate the expected +post-merge commit and pipeline. In this case, the merge train may already +have pipelines running against the previous expected post-merge commit. +These pipelines are considered redundant and will be automatically +canceled. + ## Troubleshooting ### Pipelines for merged results not created even with new change pushed to merge request diff --git a/doc/ci/multi_project_pipelines.md b/doc/ci/multi_project_pipelines.md index 377ae9717b2..9e18d1d883c 100644 --- a/doc/ci/multi_project_pipelines.md +++ b/doc/ci/multi_project_pipelines.md @@ -178,6 +178,8 @@ the ones defined in the upstream project will take precedence. ### Mirroring status from triggered pipeline +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/11238) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.3. + You can mirror the pipeline status from the triggered pipeline to the source bridge job by using `strategy: depend`. For example: diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 7569ccc04c1..c3165dc2e21 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -191,6 +191,51 @@ end policies at once. The fields for these will all have be non-nullable booleans with a default description. +## Enums + +GitLab GraphQL enums are defined in `app/graphql/types`. When defining new enums, the +following rules apply: + +- Values must be uppercase. +- Class names must end with the string `Enum`. +- The `graphql_name` must not contain the string `Enum`. + +For example: + +```ruby +module Types + class TrafficLightStateEnum < BaseEnum + graphql_name 'TrafficLightState' + description 'State of a traffic light' + + value 'RED', description: 'Drivers must stop' + value 'YELLOW', description: 'Drivers must stop when it is safe to' + value 'GREEN', description: 'Drivers can start or keep driving' + end +end +``` + +If the enum will be used for a class property in Ruby that is not an uppercase string, +you can provide a `value:` option that will adapt the uppercase value. + +In the following example: + +- GraphQL inputs of `OPENED` will be converted to `'opened'`. +- Ruby values of `'opened'` will be converted to `"OPENED"` in GraphQL responses. + +```ruby +module Types + class EpicStateEnum < BaseEnum + graphql_name 'EpicState' + description 'State of a GitLab epic' + + value 'OPENED', value: 'opened', description: 'An open Epic' + value 'CLOSED', value: 'closed', description: 'An closed Epic' + end +end + +``` + ## Authorization Authorizations can be applied to both types and fields using the same diff --git a/doc/development/architecture.md b/doc/development/architecture.md index ee5fc553e27..147bd21e6c7 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -61,8 +61,8 @@ graph TB Unicorn --> PgBouncer[PgBouncer] Unicorn --> Redis Unicorn --> Gitaly - Redis --> Sidekiq - Sidekiq["Sidekiq (GitLab Rails, ES Indexer)"] --> PgBouncer + Sidekiq --> Redis + Sidekiq --> PgBouncer GitLabWorkhorse[GitLab Workhorse] --> Unicorn GitLabWorkhorse --> Redis GitLabWorkhorse --> Gitaly diff --git a/doc/development/fe_guide/development_process.md b/doc/development/fe_guide/development_process.md index ae0e2361840..9224a2548ab 100644 --- a/doc/development/fe_guide/development_process.md +++ b/doc/development/fe_guide/development_process.md @@ -58,6 +58,24 @@ Please use your best judgement when to use it and please contribute new points t - [ ] Follow up on issues that came out of the review. Create issues for discovered edge cases that should be covered in future iterations. ``` +### Merge Request Review + +With the purpose of being [respectful of others' time](https://about.gitlab.com/handbook/values/#be-respectful-of-others-time) please follow these guidelines when asking for a review: + +- Make sure your Merge Request: + - milestone is set + - at least the labels suggested by danger-bot are set + - has a clear description + - includes before/after screenshots if there is a UI change + - pipeline is green + - includes tests + - includes a changelog entry (when necessary) +- Before assigning to a maintainer, assign to a reviewer. +- If you assigned a merge request, or pinged someone directly, keep in mind that we work in different timezones and asynchronously, so be patient. Unless the merge request is urgent (like fixing a broken master), please don't DM or reassign the merge request before waiting for a 24-hour window. +- If you have a question regarding your merge request/issue, make it on the merge request/issue. When we DM each other, we no longer have a SSOT and [no one else is able to contribute](https://about.gitlab.com/handbook/values/#public-by-default). +- When you have a big WIP merge request with many changes, you're adivsed to get the review started before adding/removing significant code. Make sure it is assigned well before the release cut-off, as the reviewer(s)/maintainer(s) would always prioritize reviewing finished MRs before WIP ones. +- Make sure to remove the WIP title before the last round of review. + ### Share your work early 1. Before writing code, ensure your vision of the architecture is aligned with diff --git a/doc/development/testing_guide/end_to_end/page_objects.md b/doc/development/testing_guide/end_to_end/page_objects.md index 850ea6b60ac..8820b54fa87 100644 --- a/doc/development/testing_guide/end_to_end/page_objects.md +++ b/doc/development/testing_guide/end_to_end/page_objects.md @@ -167,6 +167,18 @@ There are two supported methods of defining elements within a view. Any existing `.qa-selector` class should be considered deprecated and we should prefer the `data-qa-selector` method of definition. +### Exceptions + +In some cases it might not be possible or worthwhile to add a selector. + +Some UI components use external libraries, including some maintained by third parties. +Even if a library is maintained by GitLab, the selector sanity test only runs +on code within the GitLab project, so it's not possible to specify the path for +the view for code in a library. + +In such rare cases it's reasonable to use CSS selectors in page object methods, +with a comment explaining why an `element` can't be added. + ## Running the test locally During development, you can run the `qa:selectors` test by running diff --git a/doc/security/reset_root_password.md b/doc/security/reset_root_password.md index ec360e2d338..00c9dc1407d 100644 --- a/doc/security/reset_root_password.md +++ b/doc/security/reset_root_password.md @@ -9,7 +9,7 @@ To reset your root password, first log into your server with root privileges. Start a Ruby on Rails console with this command: ```bash -gitlab-rails console production +gitlab-rails console -e production ``` Wait until the console has loaded. diff --git a/doc/user/admin_area/diff_limits.md b/doc/user/admin_area/diff_limits.md index 9fe4b50a991..5117b5f476f 100644 --- a/doc/user/admin_area/diff_limits.md +++ b/doc/user/admin_area/diff_limits.md @@ -6,6 +6,8 @@ type: reference You can set a maximum size for display of diff files (patches). +For details about diff files, [View changes between files](../project/merge_requests/index.md#view-changes-between-file-versions). + ## Maximum diff patch size Diff files which exceed this value will be presented as 'too large' and won't diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index fbc130689e0..15a21bb82e0 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -205,6 +205,7 @@ Some analyzers can be customized with environment variables. | `GRADLE_PATH` | spotbugs | Path to the `gradle` executable. | | `JAVA_OPTS` | spotbugs | Additional arguments for the `java` executable. | | `JAVA_PATH` | spotbugs | Path to the `java` executable. | +| `SAST_JAVA_VERSION` | spotbugs | Which Java version to use. Supported versions are `8` and `11`. Defaults to `8`. | | `MAVEN_CLI_OPTS` | spotbugs | Additional arguments for the `mvn` or `mvnw` executable. | | `MAVEN_PATH` | spotbugs | Path to the `mvn` executable. | | `MAVEN_REPO_PATH` | spotbugs | Path to the Maven local repository (shortcut for the `maven.repo.local` property). | diff --git a/doc/user/clusters/applications.md b/doc/user/clusters/applications.md index 40ed0db4c57..f922b8911ae 100644 --- a/doc/user/clusters/applications.md +++ b/doc/user/clusters/applications.md @@ -82,10 +82,12 @@ certificates are valid and up-to-date. NOTE: **Note:** The -[stable/cert-manager](https://github.com/helm/charts/tree/master/stable/cert-manager) +[jetstack/cert-manager](https://github.com/jetstack/cert-manager) chart is used to install this application with a [`values.yaml`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/vendor/cert_manager/values.yaml) -file. +file. Prior to GitLab 12.3, +the [stable/cert-manager](https://github.com/helm/charts/tree/master/stable/cert-manager) +chart was used. ### GitLab Runner @@ -186,7 +188,8 @@ You can clone repositories from the files tab in Jupyter: ### Knative -> Available for project-level clusters since GitLab 11.5. +> - Available for project-level clusters since GitLab 11.5. +> - Available for group-level and instance-level clusters since GitLab 12.3. [Knative](https://cloud.google.com/knative) provides a platform to create, deploy, and manage serverless workloads from a Kubernetes diff --git a/doc/user/project/import/img/import_projects_from_repo_url.png b/doc/user/project/import/img/import_projects_from_repo_url.png Binary files differindex c453c7e558a..90bcff5d31b 100644 --- a/doc/user/project/import/img/import_projects_from_repo_url.png +++ b/doc/user/project/import/img/import_projects_from_repo_url.png diff --git a/doc/user/project/merge_requests/browser_performance_testing.md b/doc/user/project/merge_requests/browser_performance_testing.md index 49b9826a52a..2339cfa0db8 100644 --- a/doc/user/project/merge_requests/browser_performance_testing.md +++ b/doc/user/project/merge_requests/browser_performance_testing.md @@ -4,8 +4,7 @@ type: reference, howto # Browser Performance Testing **(PREMIUM)** -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3507) -in [GitLab Premium](https://about.gitlab.com/pricing/) 10.3. +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3507) in [GitLab Premium](https://about.gitlab.com/pricing/) 10.3. If your application offers a web interface and you are using [GitLab CI/CD](../../../ci/README.md), you can quickly determine the performance @@ -25,18 +24,20 @@ for [additional metrics](https://gitlab.com/gitlab-org/gitlab-ee/issues/4370) in a future release. Going a step further, GitLab can show the Performance report right -in the merge request widget area: +in the merge request widget area (see below). ## Use cases For instance, consider the following workflow: -1. A member of the marketing team is attempting to track engagement by adding a new tool -1. With browser performance metrics, they see how their changes are impacting the usability of the page for end users -1. The metrics show that after their changes the performance score of the page has gone down -1. When looking at the detailed report, they see that the new Javascript library was included in `<head>` which affects loading page speed -1. They ask a front end developer to help them, who sets the library to load asynchronously -1. The frontend developer approves the merge request and authorizes its deployment to production +1. A member of the marketing team is attempting to track engagement by adding a new tool. +1. With browser performance metrics, they see how their changes are impacting the usability + of the page for end users. +1. The metrics show that after their changes the performance score of the page has gone down. +1. When looking at the detailed report, they see that the new JavaScript library was + included in `<head>` which affects loading page speed. +1. They ask a front end developer to help them, who sets the library to load asynchronously. +1. The frontend developer approves the merge request and authorizes its deployment to production. ## How it works @@ -48,15 +49,165 @@ example on [Testing Browser Performance](../../../ci/examples/browser_performanc GitLab then checks this report, compares key performance metrics for each page between the source and target branches, and shows the information right on the merge request. ->**Note:** +NOTE: **Note:** If the Performance report doesn't have anything to compare to, no information will be displayed in the merge request area. That is the case when you add the Performance job in your `.gitlab-ci.yml` for the very first time. -Consecutive merge requests will have something to compare to and the Performance +Consecutive merge requests will have something to compare to, and the Performance report will be shown properly.  +## Configuring Browser Performance Testing + +NOTE: **Note:** +The job definition shown below is supported in GitLab 11.5 and later versions. +It also requires GitLab Runner 11.5 or later. For earlier versions, use the +[previous job definitions](#previous-job-definitions). + +This example shows how to run the [sitespeed.io container](https://hub.docker.com/r/sitespeedio/sitespeed.io/) +on your code by using GitLab CI/CD and [sitespeed.io](https://www.sitespeed.io) +using Docker-in-Docker. + +First, you need GitLab Runner with +[docker-in-docker build](../../../ci/docker/using_docker_build.md#use-docker-in-docker-workflow-with-docker-executor). + +Once you set up the Runner, add a new job to `.gitlab-ci.yml` that generates the +expected report: + +```yaml +performance: + stage: performance + image: docker:git + variables: + URL: https://example.com + services: + - docker:stable-dind + script: + - mkdir gitlab-exporter + - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js + - mkdir sitespeed-results + - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:6.3.1 --plugins.add ./gitlab-exporter --outputFolder sitespeed-results $URL + - mv sitespeed-results/data/performance.json performance.json + artifacts: + paths: + - sitespeed-results/ + reports: + performance: performance.json +``` + +The above example will create a `performance` job in your CI/CD pipeline and will run +sitespeed.io against the webpage you defined in `URL` to gather key metrics. +The [GitLab plugin for sitespeed.io](https://gitlab.com/gitlab-org/gl-performance) +is downloaded in order to save the report as a [Performance report artifact](../../../ci/yaml/README.md#artifactsreportsperformance-premium) +that you can later download and analyze. Due to implementation limitations we always +take the latest Performance artifact available. + +The full HTML sitespeed.io report will also be saved as an artifact, and if you have +[GitLab Pages](../pages/index.md) enabled, it can be viewed directly in your browser. + +For further customization options for sitespeed.io, including the ability to provide a +list of URLs to test, please see the [Sitespeed.io Configuration](https://www.sitespeed.io/documentation/sitespeed.io/configuration/) +documentation. + +TIP: **Tip:** +Key metrics are automatically extracted and shown in the merge request widget. + +### Performance testing on Review Apps + +The above CI YML is great for testing against static environments, and it can +be extended for dynamic environments. There are a few extra steps to take to +set this up: + +1. The `performance` job should run after the dynamic environment has started. +1. In the `review` job, persist the hostname and upload it as an artifact so + it's available to the `performance` job (the same can be done for static + environments like staging and production to unify the code path). Saving it + as an artifact is as simple as `echo $CI_ENVIRONMENT_URL > environment_url.txt` + in your job's `script`. +1. In the `performance` job, read the previous artifact into an environment + variable, like `$CI_ENVIRONMENT_URL`, and use it to parameterize the test + URLs. +1. You can now run the sitespeed.io container against the desired hostname and + paths. + +Your `.gitlab-ci.yml` file would look like: + +```yaml +stages: + - deploy + - performance + +review: + stage: deploy + environment: + name: review/$CI_COMMIT_REF_SLUG + url: http://$CI_COMMIT_REF_SLUG.$APPS_DOMAIN + script: + - run_deploy_script + - echo $CI_ENVIRONMENT_URL > environment_url.txt + artifacts: + paths: + - environment_url.txt + only: + - branches + except: + - master + +performance: + stage: performance + image: docker:git + services: + - docker:stable-dind + dependencies: + - review + script: + - export CI_ENVIRONMENT_URL=$(cat environment_url.txt) + - mkdir gitlab-exporter + - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js + - mkdir sitespeed-results + - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:6.3.1 --plugins.add ./gitlab-exporter --outputFolder sitespeed-results "$CI_ENVIRONMENT_URL" + - mv sitespeed-results/data/performance.json performance.json + artifacts: + paths: + - sitespeed-results/ + reports: + performance: performance.json +``` + +A complete example can be found in our [Auto DevOps CI YML](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml). + +### Previous job definitions + +CAUTION: **Caution:** +Before GitLab 11.5, Performance job and artifact had to be named specifically +to automatically extract report data and show it in the merge request widget. +While these old job definitions are still maintained they have been deprecated +and may be removed in next major release, GitLab 12.0. +You are advised to update your current `.gitlab-ci.yml` configuration to reflect that change. + +For GitLab 11.4 and earlier, the job should look like: + +```yaml +performance: + stage: performance + image: docker:git + variables: + URL: https://example.com + services: + - docker:stable-dind + script: + - mkdir gitlab-exporter + - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js + - mkdir sitespeed-results + - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:6.3.1 --plugins.add ./gitlab-exporter --outputFolder sitespeed-results $URL + - mv sitespeed-results/data/performance.json performance.json + artifacts: + paths: + - performance.json + - sitespeed-results/ +``` + <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/doc/user/project/merge_requests/img/merge_request_diff_v12_2.png b/doc/user/project/merge_requests/img/merge_request_diff_v12_2.png Binary files differnew file mode 100644 index 00000000000..e56fbb9750f --- /dev/null +++ b/doc/user/project/merge_requests/img/merge_request_diff_v12_2.png diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index d6da8cb99c7..4e79d66df47 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -164,6 +164,26 @@ you hide threads that are no longer relevant. [Read more about resolving threads in merge requests reviews.](../../discussions/index.md) +## View changes between file versions + +The **Changes** tab of a merge request shows the changes to files between branches or +commits. This view of changes to a file is also known as a **diff**. By default, the diff view +compares the file in the merge request branch and the file in the target branch. + +The diff view includes the following: + +- The file's name and path. +- The number of lines added and deleted. +- Buttons for the following options: + - Toggle comments for this file; useful for inline reviews. + - Edit the file in the merge request's branch. + - Show full file, in case you want to look at the changes in context with the rest of the file. + - View file at the current commit. + - Preview the changes with [Review Apps](../../../ci/review_apps/index.md). +- The changed lines, with the specific changes highlighted. + + + ## Commenting on any file line in merge requests > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/13950) in GitLab 11.5. @@ -289,6 +309,7 @@ as pushing changes: - Set the merge request to remove the source branch when it's merged. - Set the title of the merge request to a particular title. - Set the description of the merge request to a particular description. +- Add or remove labels from the merge request. ### Create a new merge request using git push options @@ -375,6 +396,33 @@ git push -o merge_request.description="The description I want" You can also use this push option in addition to the `merge_request.create` push option. +### Add or remove labels using git push options + +You can add or remove labels from merge requests using push options. + +For example, to add two labels to an existing merge request, use the +`merge_request.label` push option: + +```sh +git push -o merge_request.label="label1" -o merge_request.label="label2" +``` + +To remove two labels from an existing merge request, use +the `merge_request.unlabel` push option: + +```sh +git push -o merge_request.unlabel="label1" -o merge_request.unlabel="label2" +``` + +You can also use these push options in addition to the +`merge_request.create` push option. + +To create a merge request and add two labels to it, use: + +```sh +git push -o merge_request.create -o merge_request.label="label1" -o merge_request.label="label2" +``` + ## Find the merge request that introduced a change > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/2383) in GitLab 10.5. diff --git a/doc/workflow/img/repository_mirroring_push_settings.png b/doc/workflow/img/repository_mirroring_push_settings.png Binary files differindex 21a6aca4526..3c0eacaa2df 100644 --- a/doc/workflow/img/repository_mirroring_push_settings.png +++ b/doc/workflow/img/repository_mirroring_push_settings.png diff --git a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md index f5593927cc2..36db4f73885 100644 --- a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md +++ b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md @@ -62,9 +62,8 @@ git commit -am "Added Debian iso" # commit the file meta data git push origin master # sync the git repo and large file to the GitLab server ``` -NOTE: **Note:** **Make sure** that `.gitattributes` is tracked by Git. Otherwise Git -LFS will not be working properly for people cloning the project. +LFS will not be working properly for people cloning the project: ```bash git add .gitattributes diff --git a/doc/workflow/repository_mirroring.md b/doc/workflow/repository_mirroring.md index 753518d0424..5bf1f484106 100644 --- a/doc/workflow/repository_mirroring.md +++ b/doc/workflow/repository_mirroring.md @@ -37,7 +37,8 @@ The following are some possible use cases for repository mirroring: ## Pushing to a remote repository **(CORE)** -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/249) in GitLab Enterprise Edition 8.7. [Moved to GitLab Core](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18715) in 10.8. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/249) in GitLab Enterprise Edition 8.7. +> - [Moved to GitLab Core](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18715) in 10.8. For an existing project, you can set up push mirroring as follows: @@ -66,7 +67,8 @@ section. ### Push only protected branches **(CORE)** -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3350) in [GitLab Starter](https://about.gitlab.com/pricing/) 10.3. [Moved to GitLab Core](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18715) in 10.8. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3350) in [GitLab Starter](https://about.gitlab.com/pricing/) 10.3. +> - [Moved to GitLab Core](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18715) in 10.8. You can choose to only push your protected branches from GitLab to your remote repository. @@ -96,7 +98,8 @@ The repository will push soon. To force a push, click the appropriate button. ## Pulling from a remote repository **(STARTER)** -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/51) in GitLab Enterprise Edition 8.2. [Added Git LFS support](https://gitlab.com/gitlab-org/gitlab-ee/issues/10871) in [GitLab Starter](https://about.gitlab.com/pricing/) 11.11. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/51) in GitLab Enterprise Edition 8.2. +> - [Added Git LFS support](https://gitlab.com/gitlab-org/gitlab-ee/issues/10871) in [GitLab Starter](https://about.gitlab.com/pricing/) 11.11. NOTE: **Note:** This feature [is available for free](https://gitlab.com/gitlab-org/gitlab-ee/issues/10361) to GitLab.com users until September 22nd, 2019. @@ -154,7 +157,8 @@ Repository mirrors are updated as Sidekiq becomes available to process them. If ### SSH authentication -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2551) for Pull mirroring in [GitLab Starter](https://about.gitlab.com/pricing/) 9.5. [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22982) for Push mirroring in [GitLab Core](https://about.gitlab.com/pricing/) 11.6 +> - [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2551) for Pull mirroring in [GitLab Starter](https://about.gitlab.com/pricing/) 9.5. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22982) for Push mirroring in [GitLab Core](https://about.gitlab.com/pricing/) 11.6 SSH authentication is mutual: diff --git a/lib/banzai/filter/asset_proxy_filter.rb b/lib/banzai/filter/asset_proxy_filter.rb index 0a9a52a73a1..8acd3917d81 100644 --- a/lib/banzai/filter/asset_proxy_filter.rb +++ b/lib/banzai/filter/asset_proxy_filter.rb @@ -44,7 +44,7 @@ module Banzai Gitlab.config.asset_proxy['enabled'] = application_settings.asset_proxy_enabled Gitlab.config.asset_proxy['url'] = application_settings.asset_proxy_url Gitlab.config.asset_proxy['secret_key'] = application_settings.asset_proxy_secret_key - Gitlab.config.asset_proxy['whitelist'] = application_settings.asset_proxy_whitelist || [Gitlab.config.gitlab.host] + Gitlab.config.asset_proxy['whitelist'] = determine_whitelist(application_settings) Gitlab.config.asset_proxy['domain_regexp'] = compile_whitelist(Gitlab.config.asset_proxy.whitelist) else Gitlab.config.asset_proxy['enabled'] = ::ApplicationSetting.defaults[:asset_proxy_enabled] @@ -57,6 +57,10 @@ module Banzai escaped = domain_list.map { |domain| Regexp.escape(domain).gsub('\*', '.*?') } Regexp.new("^(#{escaped.join('|')})$", Regexp::IGNORECASE) end + + def self.determine_whitelist(application_settings) + application_settings.asset_proxy_whitelist.presence || [Gitlab.config.gitlab.host] + end end end end diff --git a/lib/gitlab/ci/pipeline/chain/build.rb b/lib/gitlab/ci/pipeline/chain/build.rb index 164a4634d84..899df81ea5c 100644 --- a/lib/gitlab/ci/pipeline/chain/build.rb +++ b/lib/gitlab/ci/pipeline/chain/build.rb @@ -19,6 +19,7 @@ module Gitlab user: @command.current_user, pipeline_schedule: @command.schedule, merge_request: @command.merge_request, + external_pull_request: @command.external_pull_request, variables_attributes: Array(@command.variables_attributes) ) diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index afad391e8e0..58f89a6be5e 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -7,7 +7,7 @@ module Gitlab Command = Struct.new( :source, :project, :current_user, :origin_ref, :checkout_sha, :after_sha, :before_sha, :source_sha, :target_sha, - :trigger_request, :schedule, :merge_request, + :trigger_request, :schedule, :merge_request, :external_pull_request, :ignore_skip_ci, :save_incompleted, :seeds_block, :variables_attributes, :push_options, :chat_data, :allow_mirror_update diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index d30d5a38670..e2911b4e6c8 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -38,8 +38,17 @@ module Gitlab ENV['CI_PROJECT_NAME'] == 'gitlab-ee' || File.exist?('../../CHANGELOG-EE.md') end + def gitlab_helper + # Unfortunately the following does not work: + # - respond_to?(:gitlab) + # - respond_to?(:gitlab, true) + gitlab + rescue NoMethodError + nil + end + def release_automation? - gitlab.mr_author == RELEASE_TOOLS_BOT + gitlab_helper&.mr_author == RELEASE_TOOLS_BOT end def project_name @@ -83,7 +92,8 @@ module Gitlab docs: "~Documentation", # Docs are reviewed along DevOps stages, so don't need roulette for now. none: "", qa: "~QA", - test: "~test for `spec/features/*`" + test: "~test for `spec/features/*`", + engineering_productivity: "Engineering Productivity for CI config review" }.freeze CATEGORIES = { %r{\Adoc/} => :none, # To reinstate roulette for documentation, set to `:docs`. @@ -125,7 +135,8 @@ module Gitlab %r{\A(ee/)?spec/(?!javascripts|frontend)[^/]+} => :backend, %r{\A(ee/)?vendor/(?!assets)[^/]+} => :backend, %r{\A(ee/)?vendor/(languages\.yml|licenses\.csv)\z} => :backend, - %r{\A(Dangerfile|Gemfile|Gemfile.lock|Procfile|Rakefile|\.gitlab-ci\.yml)\z} => :backend, + %r{\A(\.gitlab-ci\.yml\z|\.gitlab\/ci)} => :engineering_productivity, + %r{\A(Dangerfile|Gemfile|Gemfile.lock|Procfile|Rakefile)\z} => :backend, %r{\A[A-Z_]+_VERSION\z} => :backend, %r{\A\.rubocop(_todo)?\.yml\z} => :backend, diff --git a/lib/gitlab/danger/teammate.rb b/lib/gitlab/danger/teammate.rb index 2789706aa3b..4ad66f61c2b 100644 --- a/lib/gitlab/danger/teammate.rb +++ b/lib/gitlab/danger/teammate.rb @@ -42,6 +42,8 @@ module Gitlab area = role[/Test Automation Engineer(?:.*?, (\w+))/, 1] area && labels.any?("devops::#{area.downcase}") if kind == :reviewer + when :engineering_productivity + role[/Engineering Productivity/] if kind == :reviewer else capabilities(project).include?("#{kind} #{category}") end diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index bd0f3e70749..40acb24d191 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -64,6 +64,8 @@ project_tree: - :push_event_payload - stages: - :statuses + - :external_pull_request + - :external_pull_requests - :auto_devops - :triggers - :pipeline_schedules diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb index 91fe4e5d074..685cf53f27f 100644 --- a/lib/gitlab/import_export/project_tree_restorer.rb +++ b/lib/gitlab/import_export/project_tree_restorer.rb @@ -107,7 +107,7 @@ module Gitlab def project_params @project_params ||= begin - attrs = json_params.merge(override_params).merge(visibility_level) + attrs = json_params.merge(override_params).merge(visibility_level, external_label) # Cleaning all imported and overridden params Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: attrs, @@ -135,6 +135,13 @@ module Gitlab { 'visibility_level' => level } end + def external_label + label = override_params['external_authorization_classification_label'].presence || + json_params['external_authorization_classification_label'].presence + + { 'external_authorization_classification_label' => label } + end + # Given a relation hash containing one or more models and its relationships, # loops through each model and each object from a model type and # and assigns its correspondent attributes hash from +tree_hash+ diff --git a/lib/gitlab/patch/chronic_duration.rb b/lib/gitlab/patch/chronic_duration.rb new file mode 100644 index 00000000000..ab3cba3657f --- /dev/null +++ b/lib/gitlab/patch/chronic_duration.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +# Fixes a bug where parsing months doesn't take into account +# the ChronicDuration.days_per_week setting +# +# We can remove this when we do a refactor and push upstream in +# https://gitlab.com/gitlab-org/gitlab-ce/issues/66637 + +module Gitlab + module Patch + module ChronicDuration + extend ActiveSupport::Concern + + class_methods do + def duration_units_seconds_multiplier(unit) + return 0 unless duration_units_list.include?(unit) + + case unit + when 'months' + 3600 * ::ChronicDuration.hours_per_day * ::ChronicDuration.days_per_month + else + super + end + end + + # ChronicDuration#output uses 1mo = 4w as the conversion so we do the same here. + # We do need to add a special case for the default days_per_week value because + # we want to retain existing behavior for the default case + def days_per_month + ::ChronicDuration.days_per_week == 7 ? 30 : ::ChronicDuration.days_per_week * 4 + end + end + end + end +end diff --git a/lib/gitlab/push_options.rb b/lib/gitlab/push_options.rb index 682edfc4259..a2296d265cd 100644 --- a/lib/gitlab/push_options.rb +++ b/lib/gitlab/push_options.rb @@ -7,10 +7,12 @@ module Gitlab keys: [ :create, :description, + :label, :merge_when_pipeline_succeeds, :remove_source_branch, :target, - :title + :title, + :unlabel ] }, ci: { @@ -18,6 +20,11 @@ module Gitlab } }).freeze + MULTI_VALUE_OPTIONS = [ + %w[merge_request label], + %w[merge_request unlabel] + ].freeze + NAMESPACE_ALIASES = HashWithIndifferentAccess.new({ mr: :merge_request }).freeze @@ -50,12 +57,22 @@ module Gitlab next if [namespace, key].any?(&:nil?) options[namespace] ||= HashWithIndifferentAccess.new - options[namespace][key] = value + + if option_multi_value?(namespace, key) + options[namespace][key] ||= HashWithIndifferentAccess.new(0) + options[namespace][key][value] += 1 + else + options[namespace][key] = value + end end options end + def option_multi_value?(namespace, key) + MULTI_VALUE_OPTIONS.any? { |arr| arr == [namespace, key] } + end + def parse_option(option) parts = OPTION_MATCHER.match(option) return unless parts diff --git a/lib/gitlab_danger.rb b/lib/gitlab_danger.rb new file mode 100644 index 00000000000..b4768a9546d --- /dev/null +++ b/lib/gitlab_danger.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +class GitlabDanger + LOCAL_RULES ||= %w[ + changes_size + gemfile + documentation + frozen_string + duplicate_yarn_dependencies + prettier + eslint + database + ].freeze + + CI_ONLY_RULES ||= %w[ + metadata + changelog + specs + commit_messages + roulette + single_codebase + gitlab_ui_wg + ce_ee_vue_templates + only_documentation + ].freeze + + MESSAGE_PREFIX = '==>'.freeze + + attr_reader :gitlab_danger_helper + + def initialize(gitlab_danger_helper) + @gitlab_danger_helper = gitlab_danger_helper + end + + def self.local_warning_message + "#{MESSAGE_PREFIX} Only the following Danger rules can be run locally: #{LOCAL_RULES.join(', ')}" + end + + def self.success_message + "#{MESSAGE_PREFIX} No Danger rule violations!" + end + + def rule_names + ci? ? LOCAL_RULES | CI_ONLY_RULES : LOCAL_RULES + end + + def html_link(str) + self.ci? ? gitlab_danger_helper.html_link(str) : str + end + + def ci? + !gitlab_danger_helper.nil? + end +end diff --git a/lib/tasks/gitlab_danger.rake b/lib/tasks/gitlab_danger.rake new file mode 100644 index 00000000000..e75539f048c --- /dev/null +++ b/lib/tasks/gitlab_danger.rake @@ -0,0 +1,17 @@ +desc 'Run local Danger rules' +task :danger_local do + require 'gitlab_danger' + require 'gitlab/popen' + + puts("#{GitlabDanger.local_warning_message}\n") + + # _status will _always_ be 0, regardless of failure or success :( + output, _status = Gitlab::Popen.popen(%w{danger dry_run}) + + if output.empty? + puts(GitlabDanger.success_message) + else + puts(output) + exit(1) + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e15000b5184..d26ce9fa911 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7209,7 +7209,7 @@ msgstr "" msgid "Monday" msgstr "" -msgid "Monitor your errors by integrating with Sentry" +msgid "Monitor your errors by integrating with Sentry." msgstr "" msgid "Monitoring" @@ -13023,6 +13023,9 @@ msgid_plural "When these merge requests are accepted" msgstr[0] "" msgstr[1] "" +msgid "When using the <code>http://</code> or <code>https://</code> protocols, please provide the exact URL to the repository. HTTP redirects will not be followed." +msgstr "" + msgid "When:" msgstr "" diff --git a/package.json b/package.json index 23e611ae6cc..1c3eb409d9b 100644 --- a/package.json +++ b/package.json @@ -37,8 +37,8 @@ "@babel/plugin-syntax-dynamic-import": "^7.2.0", "@babel/plugin-syntax-import-meta": "^7.2.0", "@babel/preset-env": "^7.5.5", - "@gitlab/svgs": "^1.71.0", - "@gitlab/ui": "5.21.0", + "@gitlab/svgs": "^1.72.0", + "@gitlab/ui": "5.21.1", "@gitlab/visual-review-tools": "1.0.1", "apollo-cache-inmemory": "^1.5.1", "apollo-client": "^2.5.1", diff --git a/plugins/.gitignore b/plugins/.gitignore new file mode 100644 index 00000000000..e4ccdc9e2ec --- /dev/null +++ b/plugins/.gitignore @@ -0,0 +1,5 @@ +* +!*/ +!.gitignore +!.gitkeep +!examples/* diff --git a/qa/qa/page/main/menu.rb b/qa/qa/page/main/menu.rb index e3039149ab4..4676dc8d077 100644 --- a/qa/qa/page/main/menu.rb +++ b/qa/qa/page/main/menu.rb @@ -61,6 +61,10 @@ module QA end end + def sign_out_if_signed_in + sign_out if has_personal_area?(wait: 0) + end + def click_settings_link retry_until(reload: false) do within_user_menu do diff --git a/qa/qa/page/merge_request/new.rb b/qa/qa/page/merge_request/new.rb index 67459208c8d..f877ba76b38 100644 --- a/qa/qa/page/merge_request/new.rb +++ b/qa/qa/page/merge_request/new.rb @@ -64,3 +64,5 @@ module QA end end end + +QA::Page::MergeRequest::New.prepend_if_ee('QA::EE::Page::MergeRequest::New') diff --git a/qa/qa/resource/group.rb b/qa/qa/resource/group.rb index 44d9dc8f296..b5beba64c61 100644 --- a/qa/qa/resource/group.rb +++ b/qa/qa/resource/group.rb @@ -10,6 +10,7 @@ module QA end attribute :id + attribute :name def initialize @path = Runtime::Namespace.name @@ -47,6 +48,11 @@ module QA super end + def add_member(user, access_level = '30') + # 30 = developer access + post Runtime::API::Request.new(api_client, api_members_path).url, { user_id: user.id, access_level: access_level } + end + def api_get_path "/groups/#{CGI.escape("#{sandbox.path}/#{path}")}" end diff --git a/qa/qa/resource/merge_request.rb b/qa/qa/resource/merge_request.rb index 45ab2396a04..53126c67ba3 100644 --- a/qa/qa/resource/merge_request.rb +++ b/qa/qa/resource/merge_request.rb @@ -5,7 +5,8 @@ require 'securerandom' module QA module Resource class MergeRequest < Base - attr_accessor :id, + attr_accessor :approval_rules, + :id, :title, :description, :source_branch, @@ -46,6 +47,7 @@ module QA end def initialize + @approval_rules = nil @title = 'QA test - merge request' @description = 'This is a test merge request' @source_branch = "qa-test-feature-#{SecureRandom.hex(8)}" @@ -63,16 +65,17 @@ module QA project.visit! Page::Project::Show.perform(&:new_merge_request) - Page::MergeRequest::New.perform do |page| - page.fill_title(@title) - page.fill_description(@description) - page.choose_milestone(@milestone) if @milestone - page.assign_to_me if @assignee == 'me' + Page::MergeRequest::New.perform do |new| + new.fill_title(@title) + new.fill_description(@description) + new.choose_milestone(@milestone) if @milestone + new.assign_to_me if @assignee == 'me' labels.each do |label| - page.select_label(label) + new.select_label(label) end + new.add_approval_rules(approval_rules) if approval_rules - page.create_merge_request + new.create_merge_request end end diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index 4a29a14c5c2..157064dfe37 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -75,6 +75,11 @@ module QA super end + def add_member(user, access_level = '30') + # 30 = developer access + post Runtime::API::Request.new(api_client, api_members_path).url, { user_id: user.id, access_level: access_level } + end + def api_get_path "/projects/#{CGI.escape(path_with_namespace)}" end @@ -83,6 +88,10 @@ module QA "#{api_get_path}/repository/archive.#{type}" end + def api_members_path + "#{api_get_path}/members" + end + def api_post_path '/projects' end diff --git a/qa/qa/resource/user.rb b/qa/qa/resource/user.rb index eec46f46d99..911d2b2f506 100644 --- a/qa/qa/resource/user.rb +++ b/qa/qa/resource/user.rb @@ -9,6 +9,7 @@ module QA attr_writer :username, :password attr_accessor :provider, :extern_uid + attribute :id attribute :name attribute :email diff --git a/shared/.gitignore b/shared/.gitignore new file mode 100644 index 00000000000..5c0276adafd --- /dev/null +++ b/shared/.gitignore @@ -0,0 +1,4 @@ +* +!*/ +!.gitignore +!.gitkeep diff --git a/spec/factories/external_pull_requests.rb b/spec/factories/external_pull_requests.rb new file mode 100644 index 00000000000..08d0fa4d419 --- /dev/null +++ b/spec/factories/external_pull_requests.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :external_pull_request do + sequence(:pull_request_iid) + project + source_branch 'feature' + source_repository 'the-repository' + source_sha '97de212e80737a608d939f648d959671fb0a0142' + target_branch 'master' + target_repository 'the-repository' + target_sha 'a09386439ca39abe575675ffd4b89ae824fec22f' + status :open + + trait(:closed) { status 'closed' } + end +end diff --git a/spec/features/admin/clusters/applications_spec.rb b/spec/features/admin/clusters/applications_spec.rb new file mode 100644 index 00000000000..8310811b43d --- /dev/null +++ b/spec/features/admin/clusters/applications_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../../../../spec/features/clusters/installing_applications_shared_examples' + +describe 'Instance-level Cluster Applications', :js do + include GoogleApi::CloudPlatformHelpers + + let(:user) { create(:admin) } + + before do + sign_in(user) + end + + describe 'Installing applications' do + include_examples "installing applications on a cluster" do + let(:cluster_path) { admin_cluster_path(cluster) } + let(:cluster_factory_args) { [:instance] } + end + end +end diff --git a/spec/features/clusters/installing_applications_shared_examples.rb b/spec/features/clusters/installing_applications_shared_examples.rb new file mode 100644 index 00000000000..cb8fd8c607c --- /dev/null +++ b/spec/features/clusters/installing_applications_shared_examples.rb @@ -0,0 +1,228 @@ +# frozen_string_literal: true + +shared_examples "installing applications on a cluster" do + before do + visit cluster_path + end + + context 'when cluster is being created' do + let(:cluster) { create(:cluster, :providing_by_gcp, *cluster_factory_args) } + + it 'user is unable to install applications' do + expect(page).not_to have_text('Helm') + expect(page).not_to have_text('Install') + end + end + + context 'when cluster is created' do + let(:cluster) { create(:cluster, :provided_by_gcp, *cluster_factory_args) } + + it 'user can install applications' do + wait_for_requests + + page.within('.js-cluster-application-row-helm') do + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to be_nil + expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Install') + end + end + + context 'when user installs Helm' do + before do + allow(ClusterInstallAppWorker).to receive(:perform_async) + + page.within('.js-cluster-application-row-helm') do + page.find(:css, '.js-cluster-application-install-button').click + end + + wait_for_requests + end + + it 'shows the status transition' do + page.within('.js-cluster-application-row-helm') do + # FE sends request and gets the response, then the buttons is "Installing" + expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') + + Clusters::Cluster.last.application_helm.make_installing! + + # FE starts polling and update the buttons to "Installing" + expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') + + Clusters::Cluster.last.application_helm.make_installed! + + expect(page).not_to have_css('button', exact_text: 'Install', visible: :all) + expect(page).not_to have_css('button', exact_text: 'Installing', visible: :all) + expect(page).to have_css('.js-cluster-application-uninstall-button:not([disabled])', exact_text: 'Uninstall') + end + + expect(page).to have_content('Helm Tiller was successfully installed on your Kubernetes cluster') + end + end + + context 'when user installs Knative' do + before do + create(:clusters_applications_helm, :installed, cluster: cluster) + end + + context 'on an abac cluster' do + let(:cluster) { create(:cluster, :provided_by_gcp, :rbac_disabled, *cluster_factory_args) } + + it 'shows info block and not be installable' do + page.within('.js-cluster-application-row-knative') do + expect(page).to have_css('.rbac-notice') + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + end + end + end + + context 'on an rbac cluster' do + let(:cluster) { create(:cluster, :provided_by_gcp, *cluster_factory_args) } + + it 'does not show callout block and be installable' do + page.within('.js-cluster-application-row-knative') do + expect(page).not_to have_css('p', text: 'You must have an RBAC-enabled cluster', visible: :all) + expect(page).to have_css('.js-cluster-application-install-button:not([disabled])') + end + end + + describe 'when user clicks install button' do + def domainname_form_value + page.find('.js-knative-domainname').value + end + + before do + allow(ClusterInstallAppWorker).to receive(:perform_async) + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) + + page.within('.js-cluster-application-row-knative') do + expect(page).to have_css('.js-cluster-application-install-button:not([disabled])') + + page.find('.js-knative-domainname').set("domain.example.org") + + click_button 'Install' + + wait_for_requests + + expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installing') + + Clusters::Cluster.last.application_knative.make_installing! + Clusters::Cluster.last.application_knative.make_installed! + Clusters::Cluster.last.application_knative.update_attribute(:external_ip, '127.0.0.1') + end + end + + it 'shows status transition' do + page.within('.js-cluster-application-row-knative') do + expect(domainname_form_value).to eq('domain.example.org') + expect(page).to have_css('.js-cluster-application-uninstall-button', exact_text: 'Uninstall') + end + + expect(page).to have_content('Knative was successfully installed on your Kubernetes cluster') + expect(page).to have_css('.js-knative-save-domain-button'), exact_text: 'Save changes' + end + + it 'can then update the domain' do + page.within('.js-cluster-application-row-knative') do + expect(ClusterPatchAppWorker).to receive(:perform_async) + + expect(domainname_form_value).to eq('domain.example.org') + + page.find('.js-knative-domainname').set("new.domain.example.org") + + click_button 'Save changes' + + wait_for_requests + + expect(domainname_form_value).to eq('new.domain.example.org') + end + end + end + end + end + + context 'when user installs Cert Manager' do + before do + allow(ClusterInstallAppWorker).to receive(:perform_async) + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) + + create(:clusters_applications_helm, :installed, cluster: cluster) + + page.within('.js-cluster-application-row-cert_manager') do + click_button 'Install' + end + end + + it 'shows status transition' do + def email_form_value + page.find('.js-email').value + end + + page.within('.js-cluster-application-row-cert_manager') do + expect(email_form_value).to eq(cluster.user.email) + expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installing') + + page.find('.js-email').set("new_email@example.org") + Clusters::Cluster.last.application_cert_manager.make_installing! + + expect(email_form_value).to eq('new_email@example.org') + expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installing') + + Clusters::Cluster.last.application_cert_manager.make_installed! + + expect(email_form_value).to eq('new_email@example.org') + expect(page).to have_css('.js-cluster-application-uninstall-button', exact_text: 'Uninstall') + end + + expect(page).to have_content('Cert-Manager was successfully installed on your Kubernetes cluster') + end + end + + context 'when user installs Ingress' do + before do + allow(ClusterInstallAppWorker).to receive(:perform_async) + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) + + create(:clusters_applications_helm, :installed, cluster: cluster) + + page.within('.js-cluster-application-row-ingress') do + expect(page).to have_css('.js-cluster-application-install-button:not([disabled])') + page.find(:css, '.js-cluster-application-install-button').click + + wait_for_requests + end + end + + it 'shows the status transition' do + page.within('.js-cluster-application-row-ingress') do + # FE sends request and gets the response, then the buttons is "Installing" + expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') + + Clusters::Cluster.last.application_ingress.make_installing! + + # FE starts polling and update the buttons to "Installing" + expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') + + # The application becomes installed but we keep waiting for external IP address + Clusters::Cluster.last.application_ingress.make_installed! + + expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installed') + expect(page).to have_selector('.js-no-endpoint-message') + expect(page).to have_selector('.js-ingress-ip-loading-icon') + + # We receive the external IP address and display + Clusters::Cluster.last.application_ingress.update!(external_ip: '192.168.1.100') + + expect(page).not_to have_css('button', exact_text: 'Install', visible: :all) + expect(page).not_to have_css('button', exact_text: 'Installing', visible: :all) + expect(page).to have_css('.js-cluster-application-uninstall-button:not([disabled])', exact_text: 'Uninstall') + expect(page).not_to have_css('p', text: 'The endpoint is in the process of being assigned', visible: :all) + expect(page.find('.js-endpoint').value).to eq('192.168.1.100') + end + + expect(page).to have_content('Ingress was successfully installed on your Kubernetes cluster') + end + end + end +end diff --git a/spec/features/groups/clusters/applications_spec.rb b/spec/features/groups/clusters/applications_spec.rb new file mode 100644 index 00000000000..5d48df234eb --- /dev/null +++ b/spec/features/groups/clusters/applications_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../../../../spec/features/clusters/installing_applications_shared_examples' + +describe 'Group-level Cluster Applications', :js do + include GoogleApi::CloudPlatformHelpers + + let(:group) { create(:group) } + let(:user) { create(:user) } + + before do + group.add_maintainer(user) + sign_in(user) + end + + describe 'Installing applications' do + include_examples "installing applications on a cluster" do + let(:cluster_path) { group_cluster_path(group, cluster) } + let(:cluster_factory_args) { [:group, groups: [group]] } + end + end +end diff --git a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb index 6262f1ce055..c42eb8560a4 100644 --- a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb +++ b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb @@ -64,7 +64,7 @@ describe 'Merge request > User selects branches for new MR', :js do click_button "Check out branch" - expect(page).to have_content 'git checkout -b orphaned-branch origin/orphaned-branch' + expect(page).to have_content 'git checkout -b "orphaned-branch" "origin/orphaned-branch"' end it 'allows filtering multiple dropdowns' do diff --git a/spec/features/projects/clusters/applications_spec.rb b/spec/features/projects/clusters/applications_spec.rb index 3d15095e2da..ce971b158a3 100644 --- a/spec/features/projects/clusters/applications_spec.rb +++ b/spec/features/projects/clusters/applications_spec.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true require 'spec_helper' +require_relative '../../../../spec/features/clusters/installing_applications_shared_examples' -describe 'Clusters Applications', :js do +describe 'Project-level Cluster Applications', :js do include GoogleApi::CloudPlatformHelpers let(:project) { create(:project) } @@ -14,229 +15,9 @@ describe 'Clusters Applications', :js do end describe 'Installing applications' do - before do - visit project_cluster_path(project, cluster) - end - - context 'when cluster is being created' do - let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } - - it 'user is unable to install applications' do - expect(page).not_to have_css('.js-cluster-application-row-helm') - expect(page).not_to have_css('.js-cluster-application-install-button') - end - end - - context 'when cluster is created' do - let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - - it 'user can install applications' do - wait_for_requests - - page.within('.js-cluster-application-row-helm') do - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to be_nil - expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Install') - end - end - - context 'when user installs Helm' do - before do - allow(ClusterInstallAppWorker).to receive(:perform_async) - - page.within('.js-cluster-application-row-helm') do - page.find(:css, '.js-cluster-application-install-button').click - end - - wait_for_requests - end - - it 'they see status transition' do - page.within('.js-cluster-application-row-helm') do - # FE sends request and gets the response, then the buttons is "Installing" - expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') - - Clusters::Cluster.last.application_helm.make_installing! - - # FE starts polling and update the buttons to "Installing" - expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') - - Clusters::Cluster.last.application_helm.make_installed! - - expect(page).not_to have_css('.js-cluster-application-install-button') - expect(page).to have_css('.js-cluster-application-uninstall-button:not([disabled])', exact_text: 'Uninstall') - end - - expect(page).to have_content('Helm Tiller was successfully installed on your Kubernetes cluster') - end - end - - context 'when user installs Knative' do - before do - create(:clusters_applications_helm, :installed, cluster: cluster) - end - - context 'on an abac cluster' do - let(:cluster) { create(:cluster, :provided_by_gcp, :rbac_disabled, projects: [project]) } - - it 'shows info block and not be installable' do - page.within('.js-cluster-application-row-knative') do - expect(page).to have_css('.rbac-notice') - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') - end - end - end - - context 'on an rbac cluster' do - let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - - it 'does not show callout block and be installable' do - page.within('.js-cluster-application-row-knative') do - expect(page).not_to have_css('.rbac-notice') - expect(page).to have_css('.js-cluster-application-install-button:not([disabled])') - end - end - - describe 'when user clicks install button' do - def domainname_form_value - page.find('.js-knative-domainname').value - end - - before do - allow(ClusterInstallAppWorker).to receive(:perform_async) - allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) - allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) - - page.within('.js-cluster-application-row-knative') do - expect(page).to have_css('.js-cluster-application-install-button:not([disabled])') - - page.find('.js-knative-domainname').set("domain.example.org") - - click_button 'Install' - - wait_for_requests - - expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installing') - - Clusters::Cluster.last.application_knative.make_installing! - Clusters::Cluster.last.application_knative.make_installed! - Clusters::Cluster.last.application_knative.update_attribute(:external_ip, '127.0.0.1') - end - end - - it 'shows status transition' do - page.within('.js-cluster-application-row-knative') do - expect(domainname_form_value).to eq('domain.example.org') - expect(page).to have_css('.js-cluster-application-uninstall-button', exact_text: 'Uninstall') - end - - expect(page).to have_content('Knative was successfully installed on your Kubernetes cluster') - expect(page).to have_css('.js-knative-save-domain-button'), exact_text: 'Save changes' - end - - it 'can then update the domain' do - page.within('.js-cluster-application-row-knative') do - expect(ClusterPatchAppWorker).to receive(:perform_async) - - expect(domainname_form_value).to eq('domain.example.org') - - page.find('.js-knative-domainname').set("new.domain.example.org") - - click_button 'Save changes' - - wait_for_requests - - expect(domainname_form_value).to eq('new.domain.example.org') - end - end - end - end - end - - context 'when user installs Cert Manager' do - before do - allow(ClusterInstallAppWorker).to receive(:perform_async) - allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) - allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) - - create(:clusters_applications_helm, :installed, cluster: cluster) - - page.within('.js-cluster-application-row-cert_manager') do - click_button 'Install' - end - end - - it 'shows status transition' do - def email_form_value - page.find('.js-email').value - end - - page.within('.js-cluster-application-row-cert_manager') do - expect(email_form_value).to eq(cluster.user.email) - expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installing') - - page.find('.js-email').set("new_email@example.org") - Clusters::Cluster.last.application_cert_manager.make_installing! - - expect(email_form_value).to eq('new_email@example.org') - expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installing') - - Clusters::Cluster.last.application_cert_manager.make_installed! - - expect(email_form_value).to eq('new_email@example.org') - expect(page).to have_css('.js-cluster-application-uninstall-button', exact_text: 'Uninstall') - end - - expect(page).to have_content('Cert-Manager was successfully installed on your Kubernetes cluster') - end - end - - context 'when user installs Ingress' do - context 'when user installs application: Ingress' do - before do - allow(ClusterInstallAppWorker).to receive(:perform_async) - allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) - allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) - - create(:clusters_applications_helm, :installed, cluster: cluster) - - page.within('.js-cluster-application-row-ingress') do - expect(page).to have_css('.js-cluster-application-install-button:not([disabled])') - page.find(:css, '.js-cluster-application-install-button').click - - wait_for_requests - end - end - - it 'they see status transition' do - page.within('.js-cluster-application-row-ingress') do - # FE sends request and gets the response, then the buttons is "Installing" - expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') - - Clusters::Cluster.last.application_ingress.make_installing! - - # FE starts polling and update the buttons to "Installing" - expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') - - # The application becomes installed but we keep waiting for external IP address - Clusters::Cluster.last.application_ingress.make_installed! - - expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installed') - expect(page).to have_selector('.js-no-endpoint-message') - expect(page).to have_selector('.js-ingress-ip-loading-icon') - - # We receive the external IP address and display - Clusters::Cluster.last.application_ingress.update!(external_ip: '192.168.1.100') - - expect(page).not_to have_css('.js-cluster-application-install-button') - expect(page).to have_css('.js-cluster-application-uninstall-button:not([disabled])', exact_text: 'Uninstall') - expect(page).not_to have_selector('.js-no-endpoint-message') - expect(page.find('.js-endpoint').value).to eq('192.168.1.100') - end - - expect(page).to have_content('Ingress was successfully installed on your Kubernetes cluster') - end - end - end + include_examples "installing applications on a cluster" do + let(:cluster_path) { project_cluster_path(project, cluster) } + let(:cluster_factory_args) { [projects: [project]] } end end end diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 879ff01f294..ef8749be0be 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -42,6 +42,24 @@ describe IssuesFinder do end end + context 'filtering by projects' do + context 'when projects are passed in a list of ids' do + let(:params) { { projects: [project1.id] } } + + it 'returns the issue belonging to the projects' do + expect(issues).to contain_exactly(issue1) + end + end + + context 'when projects are passed in a subquery' do + let(:params) { { projects: Project.id_in(project1.id) } } + + it 'returns the issue belonging to the projects' do + expect(issues).to contain_exactly(issue1) + end + end + end + context 'filtering by group_id' do let(:params) { { group_id: group.id } } @@ -49,6 +67,30 @@ describe IssuesFinder do it 'returns all group issues' do expect(issues).to contain_exactly(issue1) end + + context 'when projects outside the group are passed' do + let(:params) { { group_id: group.id, projects: [project2.id] } } + + it 'returns no issues' do + expect(issues).to be_empty + end + end + + context 'when projects of the group are passed' do + let(:params) { { group_id: group.id, projects: [project1.id] } } + + it 'returns the issue within the group and projects' do + expect(issues).to contain_exactly(issue1) + end + end + + context 'when projects of the group are passed as a subquery' do + let(:params) { { group_id: group.id, projects: Project.id_in(project1.id) } } + + it 'returns the issue within the group and projects' do + expect(issues).to contain_exactly(issue1) + end + end end context 'when include_subgroup param is true' do @@ -59,6 +101,14 @@ describe IssuesFinder do it 'returns all group and subgroup issues' do expect(issues).to contain_exactly(issue1, issue4) end + + context 'when mixed projects are passed' do + let(:params) { { group_id: group.id, projects: [project2.id, project3.id] } } + + it 'returns the issue within the group and projects' do + expect(issues).to contain_exactly(issue4) + end + end end end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 78224f0b9da..6c0bbeff4f4 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -13,7 +13,7 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request1, merge_request4, merge_request5) end - it 'filters by project' do + it 'filters by project_id' do params = { project_id: project1.id, scope: 'authored', state: 'opened' } merge_requests = described_class.new(user, params).execute @@ -21,6 +21,14 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request1) end + it 'filters by projects' do + params = { projects: [project2.id, project3.id] } + + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request3, merge_request4) + end + it 'filters by commit sha' do merge_requests = described_class.new( user, @@ -49,6 +57,16 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request5) end + + it 'filters by group projects including subgroups' do + # project3 is not in the group, so it should not return merge_request4 + projects = [project3.id, project4.id] + params = { group_id: group.id, include_subgroups: true, projects: projects } + + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request5) + end end it 'filters by non_archived' do diff --git a/spec/frontend/clusters/components/applications_spec.js b/spec/frontend/clusters/components/applications_spec.js index 221ebb143be..fbcab078993 100644 --- a/spec/frontend/clusters/components/applications_spec.js +++ b/spec/frontend/clusters/components/applications_spec.js @@ -85,11 +85,48 @@ describe('Applications', () => { }); it('renders a row for Jupyter', () => { - expect(vm.$el.querySelector('.js-cluster-application-row-jupyter')).toBeNull(); + expect(vm.$el.querySelector('.js-cluster-application-row-jupyter')).not.toBeNull(); }); it('renders a row for Knative', () => { - expect(vm.$el.querySelector('.js-cluster-application-row-knative')).toBeNull(); + expect(vm.$el.querySelector('.js-cluster-application-row-knative')).not.toBeNull(); + }); + }); + + describe('Instance cluster applications', () => { + beforeEach(() => { + vm = mountComponent(Applications, { + type: CLUSTER_TYPE.INSTANCE, + applications: APPLICATIONS_MOCK_STATE, + }); + }); + + it('renders a row for Helm Tiller', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-helm')).not.toBeNull(); + }); + + it('renders a row for Ingress', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-ingress')).not.toBeNull(); + }); + + it('renders a row for Cert-Manager', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-cert_manager')).not.toBeNull(); + }); + + it('renders a row for Prometheus', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-prometheus')).not.toBeNull(); + }); + + it('renders a row for GitLab Runner', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-runner')).not.toBeNull(); + }); + + it('renders a row for Jupyter', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-jupyter')).not.toBeNull(); + }); + + it('renders a row for Knative', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-knative')).not.toBeNull(); }); }); diff --git a/spec/frontend/error_tracking/components/error_tracking_list_spec.js b/spec/frontend/error_tracking/components/error_tracking_list_spec.js index 67e5dc399ac..ce8b8908026 100644 --- a/spec/frontend/error_tracking/components/error_tracking_list_spec.js +++ b/spec/frontend/error_tracking/components/error_tracking_list_spec.js @@ -11,19 +11,24 @@ describe('ErrorTrackingList', () => { let wrapper; let actions; - function mountComponent({ errorTrackingEnabled = true } = {}) { + function mountComponent({ + errorTrackingEnabled = true, + userCanEnableErrorTracking = true, + stubs = { + 'gl-link': GlLink, + }, + } = {}) { wrapper = shallowMount(ErrorTrackingList, { localVue, store, propsData: { indexPath: '/path', enableErrorTrackingLink: '/link', + userCanEnableErrorTracking, errorTrackingEnabled, illustrationPath: 'illustration/path', }, - stubs: { - 'gl-link': GlLink, - }, + stubs, }); } @@ -115,4 +120,23 @@ describe('ErrorTrackingList', () => { expect(wrapper.find(GlButton).exists()).toBeFalsy(); }); }); + + describe('When error tracking is disabled and user is not allowed to enable it', () => { + beforeEach(() => { + mountComponent({ + errorTrackingEnabled: false, + userCanEnableErrorTracking: false, + stubs: { + 'gl-link': GlLink, + 'gl-empty-state': GlEmptyState, + }, + }); + }); + + it('shows empty state', () => { + expect(wrapper.find('a').attributes('href')).toBe( + '/help/user/project/operations/error_tracking.html', + ); + }); + }); }); diff --git a/spec/helpers/projects/error_tracking_helper_spec.rb b/spec/helpers/projects/error_tracking_helper_spec.rb index 7516a636c93..064b3ad21cb 100644 --- a/spec/helpers/projects/error_tracking_helper_spec.rb +++ b/spec/helpers/projects/error_tracking_helper_spec.rb @@ -6,21 +6,31 @@ describe Projects::ErrorTrackingHelper do include Gitlab::Routing.url_helpers set(:project) { create(:project) } + set(:current_user) { create(:user) } describe '#error_tracking_data' do + let(:can_enable_error_tracking) { true } let(:setting_path) { project_settings_operations_path(project) } let(:index_path) do project_error_tracking_index_path(project, format: :json) end + before do + allow(helper) + .to receive(:can?) + .with(current_user, :admin_operations, project) + .and_return(can_enable_error_tracking) + end + context 'without error_tracking_setting' do it 'returns frontend configuration' do - expect(error_tracking_data(project)).to eq( + expect(helper.error_tracking_data(current_user, project)).to match( 'index-path' => index_path, + 'user-can-enable-error-tracking' => 'true', 'enable-error-tracking-link' => setting_path, 'error-tracking-enabled' => 'false', - "illustration-path" => "/images/illustrations/cluster_popover.svg" + 'illustration-path' => match_asset_path('/assets/illustrations/cluster_popover.svg') ) end end @@ -36,7 +46,7 @@ describe Projects::ErrorTrackingHelper do end it 'show error tracking enabled' do - expect(error_tracking_data(project)).to include( + expect(helper.error_tracking_data(current_user, project)).to include( 'error-tracking-enabled' => 'true' ) end @@ -48,11 +58,21 @@ describe Projects::ErrorTrackingHelper do end it 'show error tracking not enabled' do - expect(error_tracking_data(project)).to include( + expect(helper.error_tracking_data(current_user, project)).to include( 'error-tracking-enabled' => 'false' ) end end end + + context 'when user is not maintainer' do + let(:can_enable_error_tracking) { false } + + it 'shows error tracking enablement as disabled' do + expect(helper.error_tracking_data(current_user, project)).to include( + 'user-can-enable-error-tracking' => 'false' + ) + end + end end end diff --git a/spec/javascripts/boards/board_new_issue_spec.js b/spec/javascripts/boards/board_new_issue_spec.js index 721d0b8172d..76675a78db2 100644 --- a/spec/javascripts/boards/board_new_issue_spec.js +++ b/spec/javascripts/boards/board_new_issue_spec.js @@ -171,6 +171,32 @@ describe('Issue boards new issue form', () => { .then(done) .catch(done.fail); }); + + it('sets detail weight after submit', done => { + boardsStore.weightFeatureAvailable = true; + vm.title = 'submit issue'; + + Vue.nextTick() + .then(submitIssue) + .then(() => { + expect(boardsStore.detail.list.weight).toBe(list.weight); + }) + .then(done) + .catch(done.fail); + }); + + it('does not set detail weight after submit', done => { + boardsStore.weightFeatureAvailable = false; + vm.title = 'submit issue'; + + Vue.nextTick() + .then(submitIssue) + .then(() => { + expect(boardsStore.detail.list.weight).toBe(list.weight); + }) + .then(done) + .catch(done.fail); + }); }); describe('submit error', () => { diff --git a/spec/javascripts/boards/mock_data.js b/spec/javascripts/boards/mock_data.js index ea22ae5c4e7..50ad1442873 100644 --- a/spec/javascripts/boards/mock_data.js +++ b/spec/javascripts/boards/mock_data.js @@ -12,6 +12,7 @@ export const listObj = { position: 0, title: 'Test', list_type: 'label', + weight: 3, label: { id: 5000, title: 'Testing', @@ -26,6 +27,7 @@ export const listObjDuplicate = { position: 1, title: 'Test', list_type: 'label', + weight: 3, label: { id: listObj.label.id, title: 'Testing', diff --git a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb index b7f45421b2a..0c4ccbf28f4 100644 --- a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb +++ b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb @@ -36,6 +36,17 @@ describe Banzai::Filter::AssetProxyFilter do expect(Gitlab.config.asset_proxy.whitelist).to eq %w(gitlab.com *.mydomain.com) expect(Gitlab.config.asset_proxy.domain_regexp).to eq /^(gitlab\.com|.*?\.mydomain\.com)$/i end + + context 'when whitelist is empty' do + it 'defaults to the install domain' do + stub_application_setting(asset_proxy_enabled: true) + stub_application_setting(asset_proxy_whitelist: []) + + described_class.initialize_settings + + expect(Gitlab.config.asset_proxy.whitelist).to eq [Gitlab.config.gitlab.host] + end + end end context 'when properly configured' do diff --git a/spec/lib/gitlab/ci/build/policy/refs_spec.rb b/spec/lib/gitlab/ci/build/policy/refs_spec.rb index 43c5d3ec980..8fc1e0a4e88 100644 --- a/spec/lib/gitlab/ci/build/policy/refs_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/refs_spec.rb @@ -84,6 +84,20 @@ describe Gitlab::Ci::Build::Policy::Refs do .not_to be_satisfied_by(pipeline) end end + + context 'when source is external_pull_request_event' do + let(:pipeline) { build_stubbed(:ci_pipeline, source: :external_pull_request_event) } + + it 'is satisfied with only: external_pull_request' do + expect(described_class.new(%w[external_pull_requests])) + .to be_satisfied_by(pipeline) + end + + it 'is not satisfied with only: external_pull_request_event' do + expect(described_class.new(%w[external_pull_request_events])) + .not_to be_satisfied_by(pipeline) + end + end end context 'when matching a ref by a regular expression' do diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb index bf9ff922c05..ba4f841cf43 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb @@ -128,4 +128,38 @@ describe Gitlab::Ci::Pipeline::Chain::Build do expect(pipeline.target_sha).to eq(merge_request.target_branch_sha) end end + + context 'when pipeline is running for an external pull request' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :external_pull_request_event, + origin_ref: 'feature', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + source_sha: external_pull_request.source_sha, + target_sha: external_pull_request.target_sha, + trigger_request: nil, + schedule: nil, + external_pull_request: external_pull_request, + project: project, + current_user: user) + end + + let(:external_pull_request) { build(:external_pull_request, project: project) } + + before do + step.perform! + end + + it 'correctly indicated that this is an external pull request pipeline' do + expect(pipeline).to be_external_pull_request_event + expect(pipeline.external_pull_request).to eq(external_pull_request) + end + + it 'correctly sets source sha and target sha to pipeline' do + expect(pipeline.source_sha).to eq(external_pull_request.source_sha) + expect(pipeline.target_sha).to eq(external_pull_request.target_sha) + end + end end diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index 2990594c538..1b4d366ce7b 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -11,16 +11,62 @@ describe Gitlab::Danger::Helper do class FakeDanger include Gitlab::Danger::Helper - attr_reader :git + attr_reader :git, :gitlab - def initialize(git:) + def initialize(git:, gitlab:) @git = git + @gitlab = gitlab end end let(:fake_git) { double('fake-git') } - subject(:helper) { FakeDanger.new(git: fake_git) } + let(:mr_author) { nil } + let(:fake_gitlab) { double('fake-gitlab', mr_author: mr_author) } + + subject(:helper) { FakeDanger.new(git: fake_git, gitlab: fake_gitlab) } + + describe '#gitlab_helper' do + context 'when gitlab helper is not available' do + let(:fake_gitlab) { nil } + + it 'returns nil' do + expect(helper.gitlab_helper).to be_nil + end + end + + context 'when gitlab helper is available' do + it 'returns the gitlab helper' do + expect(helper.gitlab_helper).to eq(fake_gitlab) + end + end + end + + describe '#release_automation?' do + context 'when gitlab helper is not available' do + it 'returns false' do + expect(helper.release_automation?).to be_falsey + end + end + + context 'when gitlab helper is available' do + context "but the MR author isn't the RELEASE_TOOLS_BOT" do + let(:mr_author) { 'johnmarston' } + + it 'returns false' do + expect(helper.release_automation?).to be_falsey + end + end + + context 'and the MR author is the RELEASE_TOOLS_BOT' do + let(:mr_author) { described_class::RELEASE_TOOLS_BOT } + + it 'returns true' do + expect(helper.release_automation?).to be_truthy + end + end + end + end describe '#all_changed_files' do subject { helper.all_changed_files } @@ -168,9 +214,13 @@ describe Gitlab::Danger::Helper do 'Gemfile.lock' | :backend 'Procfile' | :backend 'Rakefile' | :backend - '.gitlab-ci.yml' | :backend 'FOO_VERSION' | :backend + '.gitlab-ci.yml' | :engineering_productivity + '.gitlab/ci/cng.gitlab-ci.yml' | :engineering_productivity + '.gitlab/ci/ee-specific-checks.gitlab-ci.yml' | :engineering_productivity + 'lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml' | :backend + 'ee/FOO_VERSION' | :unknown 'db/schema.rb' | :database diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb index afbc3896a70..ca036390bde 100644 --- a/spec/lib/gitlab/danger/teammate_spec.rb +++ b/spec/lib/gitlab/danger/teammate_spec.rb @@ -65,6 +65,18 @@ describe Gitlab::Danger::Teammate do expect(subject.reviewer?(project, :test, labels)).to be_falsey end end + + context 'when role is Backend Engineer, Engineering Productivity' do + let(:role) { 'Backend Engineer, Engineering Productivity' } + + it '#reviewer? returns true' do + expect(subject.reviewer?(project, :engineering_productivity, labels)).to be_truthy + end + + it '#maintainer? returns false' do + expect(subject.maintainer?(project, :engineering_productivity, labels)).to be_falsey + end + end end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 47ba7eff8ed..dafa4243145 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -23,6 +23,7 @@ issues: - epic_issue - epic - designs +- design_versions events: - author - project @@ -126,6 +127,8 @@ merge_requests: - blocks_as_blockee - blocking_merge_requests - blocked_merge_requests +external_pull_requests: +- project merge_request_diff: - merge_request - merge_request_diff_commits @@ -155,6 +158,7 @@ ci_pipelines: - pipeline_schedule - merge_requests_as_head_pipeline - merge_request +- external_pull_request - deployments - environments - chat_data @@ -402,6 +406,7 @@ project: - merge_trains - designs - project_aliases +- external_pull_requests award_emoji: - awardable - user diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 0aef4887c75..87be7857e67 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -512,6 +512,24 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(Milestone.find_by_title('Group-level milestone').iid).to eq(2) end end + + context 'with external authorization classification labels' do + it 'converts empty external classification authorization labels to nil' do + project.create_import_data(data: { override_params: { external_authorization_classification_label: "" } }) + + restored_project_json + + expect(project.external_authorization_classification_label).to be_nil + end + + it 'preserves valid external classification authorization labels' do + project.create_import_data(data: { override_params: { external_authorization_classification_label: "foobar" } }) + + restored_project_json + + expect(project.external_authorization_classification_label).to eq("foobar") + end + end end describe '#restored_project' do diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index d34c6d2421b..e9750d23c53 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -270,6 +270,7 @@ Ci::Pipeline: - protected - iid - merge_request_id +- external_pull_request_id Ci::Stage: - id - name @@ -715,3 +716,16 @@ List: - updated_at - milestone_id - user_id +ExternalPullRequest: +- id +- created_at +- updated_at +- project_id +- pull_request_iid +- status +- source_branch +- target_branch +- source_repository +- target_repository +- source_sha +- target_sha diff --git a/spec/lib/gitlab/patch/chronic_duration_spec.rb b/spec/lib/gitlab/patch/chronic_duration_spec.rb new file mode 100644 index 00000000000..541037ec1a2 --- /dev/null +++ b/spec/lib/gitlab/patch/chronic_duration_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Patch::ChronicDuration do + subject { ChronicDuration.parse('1mo') } + + it 'uses default conversions' do + expect(subject).to eq(2_592_000) + end + + context 'with custom conversions' do + before do + ChronicDuration.hours_per_day = 8 + ChronicDuration.days_per_week = 5 + end + + after do + ChronicDuration.hours_per_day = 24 + ChronicDuration.days_per_week = 7 + end + + it 'uses custom conversions' do + expect(subject).to eq(576_000) + end + end +end diff --git a/spec/lib/gitlab/time_tracking_formatter_spec.rb b/spec/lib/gitlab/time_tracking_formatter_spec.rb index a85d418777f..cfc804c13a7 100644 --- a/spec/lib/gitlab/time_tracking_formatter_spec.rb +++ b/spec/lib/gitlab/time_tracking_formatter_spec.rb @@ -17,6 +17,14 @@ describe Gitlab::TimeTrackingFormatter do it { expect(subject).to eq(-12_000) } end + + context 'durations with months' do + let(:duration_string) { '1mo' } + + it 'uses our custom conversions' do + expect(subject).to eq(576_000) + end + end end describe '#output' do diff --git a/spec/lib/gitlab_danger_spec.rb b/spec/lib/gitlab_danger_spec.rb new file mode 100644 index 00000000000..623ac20fa7c --- /dev/null +++ b/spec/lib/gitlab_danger_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabDanger do + let(:gitlab_danger_helper) { nil } + + subject { described_class.new(gitlab_danger_helper) } + + describe '.local_warning_message' do + it 'returns an informational message with rules that can run' do + expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changes_size, gemfile, documentation, frozen_string, duplicate_yarn_dependencies, prettier, eslint, database') + end + end + + describe '.success_message' do + it 'returns an informational success message' do + expect(described_class.success_message).to eq('==> No Danger rule violations!') + end + end + + describe '#rule_names' do + context 'when running locally' do + it 'returns local only rules' do + expect(subject.rule_names).to eq(described_class::LOCAL_RULES) + end + end + + context 'when running under CI' do + let(:gitlab_danger_helper) { double('danger_gitlab_helper') } + + it 'returns all rules' do + expect(subject.rule_names).to eq(described_class::LOCAL_RULES | described_class::CI_ONLY_RULES) + end + end + end + + describe '#html_link' do + context 'when running locally' do + it 'returns the same string' do + str = 'something' + + expect(subject.html_link(str)).to eq(str) + end + end + + context 'when running under CI' do + let(:gitlab_danger_helper) { double('danger_gitlab_helper') } + + it 'returns a HTML link formatted version of the string' do + str = 'something' + html_formatted_str = %Q{<a href="#{str}">#{str}</a>} + + expect(gitlab_danger_helper).to receive(:html_link).with(str).and_return(html_formatted_str) + + expect(subject.html_link(str)).to eq(html_formatted_str) + end + end + end + + describe '#ci?' do + context 'when gitlab_danger_helper is not available' do + it 'returns false' do + expect(subject.ci?).to be_falsey + end + end + + context 'when gitlab_danger_helper is available' do + let(:gitlab_danger_helper) { double('danger_gitlab_helper') } + + it 'returns true' do + expect(subject.ci?).to be_truthy + end + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 63ca383ac4b..146e479adef 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -20,6 +20,7 @@ describe Ci::Pipeline, :mailer do it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to belong_to(:pipeline_schedule) } it { is_expected.to belong_to(:merge_request) } + it { is_expected.to belong_to(:external_pull_request) } it { is_expected.to have_many(:statuses) } it { is_expected.to have_many(:trigger_requests) } @@ -885,6 +886,25 @@ describe Ci::Pipeline, :mailer do end end end + + context 'when source is external pull request' do + let(:pipeline) do + create(:ci_pipeline, source: :external_pull_request_event, external_pull_request: pull_request) + end + + let(:pull_request) { create(:external_pull_request, project: project) } + + it 'exposes external pull request pipeline variables' do + expect(subject.to_hash) + .to include( + 'CI_EXTERNAL_PULL_REQUEST_IID' => pull_request.pull_request_iid.to_s, + 'CI_EXTERNAL_PULL_REQUEST_SOURCE_BRANCH_SHA' => pull_request.source_sha, + 'CI_EXTERNAL_PULL_REQUEST_TARGET_BRANCH_SHA' => pull_request.target_sha, + 'CI_EXTERNAL_PULL_REQUEST_SOURCE_BRANCH_NAME' => pull_request.source_branch, + 'CI_EXTERNAL_PULL_REQUEST_TARGET_BRANCH_NAME' => pull_request.target_branch + ) + end + end end describe '#protected_ref?' do diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index 1b39328752d..e1eee014567 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -81,27 +81,45 @@ describe Clusters::Applications::Jupyter do end describe '#files' do - let(:application) { create(:clusters_applications_jupyter) } + let(:cluster) { create(:cluster, :with_installed_helm, :provided_by_gcp, :project) } + let(:application) { create(:clusters_applications_jupyter, cluster: cluster) } let(:values) { subject[:'values.yaml'] } subject { application.files } - it 'includes valid values' do - expect(values).to include('ingress') - expect(values).to include('hub') - expect(values).to include('rbac') - expect(values).to include('proxy') - expect(values).to include('auth') - expect(values).to include('singleuser') - expect(values).to match(/clientId: '?#{application.oauth_application.uid}/) - expect(values).to match(/callbackUrl: '?#{application.callback_url}/) - expect(values).to include("gitlabProjectIdWhitelist:\n - #{application.cluster.project.id}") - expect(values).to include("c.GitLabOAuthenticator.scope = ['api read_repository write_repository']") - expect(values).to match(/GITLAB_HOST: '?#{Gitlab.config.gitlab.host}/) + context 'when cluster belongs to a project' do + it 'includes valid values' do + expect(values).to include('ingress') + expect(values).to include('hub') + expect(values).to include('rbac') + expect(values).to include('proxy') + expect(values).to include('auth') + expect(values).to include('singleuser') + expect(values).to match(/clientId: '?#{application.oauth_application.uid}/) + expect(values).to match(/callbackUrl: '?#{application.callback_url}/) + expect(values).to include("gitlabProjectIdWhitelist:\n - #{application.cluster.project.id}") + expect(values).to include("c.GitLabOAuthenticator.scope = ['api read_repository write_repository']") + expect(values).to match(/GITLAB_HOST: '?#{Gitlab.config.gitlab.host}/) + expect(values).to match(/GITLAB_CLUSTER_ID: '?#{application.cluster.id}/) + end end - context 'when cluster belongs to a project' do - it 'sets GitLab project id' do + context 'when cluster belongs to a group' do + let(:group) { create(:group) } + let(:cluster) { create(:cluster, :with_installed_helm, :provided_by_gcp, :group, groups: [group]) } + + it 'includes valid values' do + expect(values).to include('ingress') + expect(values).to include('hub') + expect(values).to include('rbac') + expect(values).to include('proxy') + expect(values).to include('auth') + expect(values).to include('singleuser') + expect(values).to match(/clientId: '?#{application.oauth_application.uid}/) + expect(values).to match(/callbackUrl: '?#{application.callback_url}/) + expect(values).to include("gitlabGroupWhitelist:\n - #{group.to_param}") + expect(values).to include("c.GitLabOAuthenticator.scope = ['api read_repository write_repository']") + expect(values).to match(/GITLAB_HOST: '?#{Gitlab.config.gitlab.host}/) expect(values).to match(/GITLAB_CLUSTER_ID: '?#{application.cluster.id}/) end end diff --git a/spec/models/external_pull_request_spec.rb b/spec/models/external_pull_request_spec.rb new file mode 100644 index 00000000000..e85d5b2f6c7 --- /dev/null +++ b/spec/models/external_pull_request_spec.rb @@ -0,0 +1,220 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ExternalPullRequest do + let(:project) { create(:project) } + let(:source_branch) { 'the-branch' } + let(:status) { :open } + + it { is_expected.to belong_to(:project) } + + shared_examples 'has errors on' do |attribute| + it "has errors for #{attribute}" do + expect(subject).not_to be_valid + expect(subject.errors[attribute]).not_to be_empty + end + end + + describe 'validations' do + context 'when source branch not present' do + subject { build(:external_pull_request, source_branch: nil) } + + it_behaves_like 'has errors on', :source_branch + end + + context 'when status not present' do + subject { build(:external_pull_request, status: nil) } + + it_behaves_like 'has errors on', :status + end + + context 'when pull request is from a fork' do + subject { build(:external_pull_request, source_repository: 'the-fork', target_repository: 'the-target') } + + it_behaves_like 'has errors on', :base + end + end + + describe 'create_or_update_from_params' do + subject { described_class.create_or_update_from_params(params) } + + context 'when pull request does not exist' do + context 'when params are correct' do + let(:params) do + { + project_id: project.id, + pull_request_iid: 123, + source_branch: 'feature', + target_branch: 'master', + source_repository: 'the-repository', + target_repository: 'the-repository', + source_sha: '97de212e80737a608d939f648d959671fb0a0142', + target_sha: 'a09386439ca39abe575675ffd4b89ae824fec22f', + status: :open + } + end + + it 'saves the model successfully and returns it' do + expect(subject).to be_persisted + expect(subject).to be_valid + end + + it 'yields the model' do + yielded_value = nil + + result = described_class.create_or_update_from_params(params) do |pull_request| + yielded_value = pull_request + end + + expect(result).to eq(yielded_value) + end + end + + context 'when params are not correct' do + let(:params) do + { + pull_request_iid: 123, + source_branch: 'feature', + target_branch: 'master', + source_repository: 'the-repository', + target_repository: 'the-repository', + source_sha: nil, + target_sha: nil, + status: :open + } + end + + it 'returns an invalid model' do + expect(subject).not_to be_persisted + expect(subject).not_to be_valid + end + end + end + + context 'when pull request exists' do + let!(:pull_request) do + create(:external_pull_request, + project: project, + source_sha: '97de212e80737a608d939f648d959671fb0a0142') + end + + context 'when params are correct' do + let(:params) do + { + pull_request_iid: pull_request.pull_request_iid, + source_branch: pull_request.source_branch, + target_branch: pull_request.target_branch, + source_repository: 'the-repository', + target_repository: 'the-repository', + source_sha: 'ce84140e8b878ce6e7c4d298c7202ff38170e3ac', + target_sha: pull_request.target_sha, + status: :open + } + end + + it 'updates the model successfully and returns it' do + expect(subject).to be_valid + expect(subject.source_sha).to eq(params[:source_sha]) + expect(pull_request.reload.source_sha).to eq(params[:source_sha]) + end + end + + context 'when params are not correct' do + let(:params) do + { + pull_request_iid: pull_request.pull_request_iid, + source_branch: pull_request.source_branch, + target_branch: pull_request.target_branch, + source_repository: 'the-repository', + target_repository: 'the-repository', + source_sha: nil, + target_sha: nil, + status: :open + } + end + + it 'returns an invalid model' do + expect(subject).not_to be_valid + expect(pull_request.reload.source_sha).not_to be_nil + expect(pull_request.target_sha).not_to be_nil + end + end + end + end + + describe '#open?' do + it 'returns true if status is open' do + pull_request = create(:external_pull_request, status: :open) + + expect(pull_request).to be_open + end + + it 'returns false if status is not open' do + pull_request = create(:external_pull_request, status: :closed) + + expect(pull_request).not_to be_open + end + end + + describe '#closed?' do + it 'returns true if status is closed' do + pull_request = build(:external_pull_request, status: :closed) + + expect(pull_request).to be_closed + end + + it 'returns false if status is not closed' do + pull_request = build(:external_pull_request, status: :open) + + expect(pull_request).not_to be_closed + end + end + + describe '#actual_branch_head?' do + let(:project) { create(:project, :repository) } + let(:branch) { project.repository.branches.first } + let(:source_branch) { branch.name } + + let(:pull_request) do + create(:external_pull_request, + project: project, + source_branch: source_branch, + source_sha: source_sha) + end + + context 'when source sha matches the head of the branch' do + let(:source_sha) { branch.target } + + it 'returns true' do + expect(pull_request).to be_actual_branch_head + end + end + + context 'when source sha does not match the head of the branch' do + let(:source_sha) { project.repository.commit('HEAD').sha } + + it 'returns true' do + expect(pull_request).not_to be_actual_branch_head + end + end + end + + describe '#from_fork?' do + it 'returns true if source_repository differs from target_repository' do + pull_request = build(:external_pull_request, + source_repository: 'repository-1', + target_repository: 'repository-2') + + expect(pull_request).to be_from_fork + end + + it 'returns false if source_repository is the same as target_repository' do + pull_request = build(:external_pull_request, + source_repository: 'repository-1', + target_repository: 'repository-1') + + expect(pull_request).not_to be_from_fork + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index bfbcac60fea..e2a684c42ae 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -99,6 +99,7 @@ describe Project do it { is_expected.to have_many(:project_deploy_tokens) } it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) } it { is_expected.to have_many(:cycle_analytics_stages) } + it { is_expected.to have_many(:external_pull_requests) } it 'has an inverse relationship with merge requests' do expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project) diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 478f09a7881..cf459ba99c1 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -20,11 +20,6 @@ describe 'Rack Attack global throttles' do let(:period_in_seconds) { 10000 } let(:period) { period_in_seconds.seconds } - let(:url_that_does_not_require_authentication) { '/users/sign_in' } - let(:url_that_requires_authentication) { '/dashboard/snippets' } - let(:url_api_internal) { '/api/v4/internal/check' } - let(:api_partial_url) { '/todos' } - around do |example| # Instead of test environment's :null_store so the throttles can increment Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new @@ -35,112 +30,10 @@ describe 'Rack Attack global throttles' do Rack::Attack.cache.store = Rails.cache end - # Requires let variables: - # * throttle_setting_prefix (e.g. "throttle_authenticated_api" or "throttle_authenticated_web") - # * get_args - # * other_user_get_args - shared_examples_for 'rate-limited token-authenticated requests' do - before do - # Set low limits - settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period - settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds - end - - context 'when the throttle is enabled' do - before do - settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true - stub_application_setting(settings_to_set) - end - - it 'rejects requests over the rate limit' do - # At first, allow requests under the rate limit. - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - # the last straw - expect_rejection { get(*get_args) } - end - - it 'allows requests after throttling and then waiting for the next period' do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - expect_rejection { get(*get_args) } - - Timecop.travel(period.from_now) do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - expect_rejection { get(*get_args) } - end - end - - it 'counts requests from different users separately, even from the same IP' do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - # would be over the limit if this wasn't a different user - get(*other_user_get_args) - expect(response).to have_http_status 200 - end - - it 'counts all requests from the same user, even via different IPs' do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') - - expect_rejection { get(*get_args) } - end - - it 'logs RackAttack info into structured logs' do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - arguments = { - message: 'Rack_Attack', - env: :throttle, - remote_ip: '127.0.0.1', - request_method: 'GET', - path: get_args.first, - user_id: user.id, - username: user.username - } - - expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once - - expect_rejection { get(*get_args) } - end - end - - context 'when the throttle is disabled' do - before do - settings_to_set[:"#{throttle_setting_prefix}_enabled"] = false - stub_application_setting(settings_to_set) - end - - it 'allows requests over the rate limit' do - (1 + requests_per_period).times do - get(*get_args) - expect(response).to have_http_status 200 - end - end - end - end - describe 'unauthenticated requests' do + let(:url_that_does_not_require_authentication) { '/users/sign_in' } + let(:url_api_internal) { '/api/v4/internal/check' } + before do # Set low limits settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period @@ -245,6 +138,7 @@ describe 'Rack Attack global throttles' do let(:other_user) { create(:user) } let(:other_user_token) { create(:personal_access_token, user: other_user) } let(:throttle_setting_prefix) { 'throttle_authenticated_api' } + let(:api_partial_url) { '/todos' } context 'with the token in the query string' do let(:get_args) { [api(api_partial_url, personal_access_token: token)] } @@ -265,10 +159,13 @@ describe 'Rack Attack global throttles' do let(:user) { create(:user) } let(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) } let(:token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api") } + let(:other_user) { create(:user) } let(:other_user_application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: other_user) } let(:other_user_token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: other_user.id, scopes: "api") } + let(:throttle_setting_prefix) { 'throttle_authenticated_api' } + let(:api_partial_url) { '/todos' } context 'with the token in the query string' do let(:get_args) { [api(api_partial_url, oauth_access_token: token)] } @@ -299,110 +196,11 @@ describe 'Rack Attack global throttles' do end describe 'web requests authenticated with regular login' do + let(:throttle_setting_prefix) { 'throttle_authenticated_web' } let(:user) { create(:user) } + let(:url_that_requires_authentication) { '/dashboard/snippets' } - before do - login_as(user) - - # Set low limits - settings_to_set[:throttle_authenticated_web_requests_per_period] = requests_per_period - settings_to_set[:throttle_authenticated_web_period_in_seconds] = period_in_seconds - end - - context 'when the throttle is enabled' do - before do - settings_to_set[:throttle_authenticated_web_enabled] = true - stub_application_setting(settings_to_set) - end - - it 'rejects requests over the rate limit' do - # At first, allow requests under the rate limit. - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - # the last straw - expect_rejection { get url_that_requires_authentication } - end - - it 'allows requests after throttling and then waiting for the next period' do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - expect_rejection { get url_that_requires_authentication } - - Timecop.travel(period.from_now) do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - expect_rejection { get url_that_requires_authentication } - end - end - - it 'counts requests from different users separately, even from the same IP' do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - # would be over the limit if this wasn't a different user - login_as(create(:user)) - - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - it 'counts all requests from the same user, even via different IPs' do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') - - expect_rejection { get url_that_requires_authentication } - end - - it 'logs RackAttack info into structured logs' do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - arguments = { - message: 'Rack_Attack', - env: :throttle, - remote_ip: '127.0.0.1', - request_method: 'GET', - path: '/dashboard/snippets', - user_id: user.id, - username: user.username - } - - expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once - - get url_that_requires_authentication - end - end - - context 'when the throttle is disabled' do - before do - settings_to_set[:throttle_authenticated_web_enabled] = false - stub_application_setting(settings_to_set) - end - - it 'allows requests over the rate limit' do - (1 + requests_per_period).times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - end - end + it_behaves_like 'rate-limited web authenticated requests' end def api_get_args_with_token_headers(partial_url, token_headers) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 6cec93a53fd..d8880819d9f 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -23,6 +23,7 @@ describe Ci::CreatePipelineService do trigger_request: nil, variables_attributes: nil, merge_request: nil, + external_pull_request: nil, push_options: nil, source_sha: nil, target_sha: nil, @@ -36,8 +37,11 @@ describe Ci::CreatePipelineService do source_sha: source_sha, target_sha: target_sha } - described_class.new(project, user, params).execute( - source, save_on_errors: save_on_errors, trigger_request: trigger_request, merge_request: merge_request) + described_class.new(project, user, params).execute(source, + save_on_errors: save_on_errors, + trigger_request: trigger_request, + merge_request: merge_request, + external_pull_request: external_pull_request) end # rubocop:enable Metrics/ParameterLists @@ -969,6 +973,152 @@ describe Ci::CreatePipelineService do end end + describe 'Pipeline for external pull requests' do + let(:pipeline) do + execute_service(source: source, + external_pull_request: pull_request, + ref: ref_name, + source_sha: source_sha, + target_sha: target_sha) + end + + before do + stub_ci_pipeline_yaml_file(YAML.dump(config)) + end + + let(:ref_name) { 'refs/heads/feature' } + let(:source_sha) { project.commit(ref_name).id } + let(:target_sha) { nil } + + context 'when source is external pull request' do + let(:source) { :external_pull_request_event } + + context 'when config has external_pull_requests keywords' do + let(:config) do + { + build: { + stage: 'build', + script: 'echo' + }, + test: { + stage: 'test', + script: 'echo', + only: ['external_pull_requests'] + }, + pages: { + stage: 'deploy', + script: 'echo', + except: ['external_pull_requests'] + } + } + end + + context 'when external pull request is specified' do + let(:pull_request) { create(:external_pull_request, project: project, source_branch: 'feature', target_branch: 'master') } + let(:ref_name) { pull_request.source_ref } + + it 'creates an external pull request pipeline' do + expect(pipeline).to be_persisted + expect(pipeline).to be_external_pull_request_event + expect(pipeline.external_pull_request).to eq(pull_request) + expect(pipeline.source_sha).to eq(source_sha) + expect(pipeline.builds.order(:stage_id) + .map(&:name)) + .to eq(%w[build test]) + end + + context 'when ref is tag' do + let(:ref_name) { 'refs/tags/v1.1.0' } + + it 'does not create an extrnal pull request pipeline' do + expect(pipeline).not_to be_persisted + expect(pipeline.errors[:tag]).to eq(["is not included in the list"]) + end + end + + context 'when pull request is created from fork' do + it 'does not create an external pull request pipeline' + end + + context "when there are no matched jobs" do + let(:config) do + { + test: { + stage: 'test', + script: 'echo', + except: ['external_pull_requests'] + } + } + end + + it 'does not create a detached merge request pipeline' do + expect(pipeline).not_to be_persisted + expect(pipeline.errors[:base]).to eq(["No stages / jobs for this pipeline."]) + end + end + end + + context 'when external pull request is not specified' do + let(:pull_request) { nil } + + it 'does not create an external pull request pipeline' do + expect(pipeline).not_to be_persisted + expect(pipeline.errors[:external_pull_request]).to eq(["can't be blank"]) + end + end + end + + context "when config does not have external_pull_requests keywords" do + let(:config) do + { + build: { + stage: 'build', + script: 'echo' + }, + test: { + stage: 'test', + script: 'echo' + }, + pages: { + stage: 'deploy', + script: 'echo' + } + } + end + + context 'when external pull request is specified' do + let(:pull_request) do + create(:external_pull_request, + project: project, + source_branch: Gitlab::Git.ref_name(ref_name), + target_branch: 'master') + end + + it 'creates an external pull request pipeline' do + expect(pipeline).to be_persisted + expect(pipeline).to be_external_pull_request_event + expect(pipeline.external_pull_request).to eq(pull_request) + expect(pipeline.source_sha).to eq(source_sha) + expect(pipeline.builds.order(:stage_id) + .map(&:name)) + .to eq(%w[build test pages]) + end + end + + context 'when external pull request is not specified' do + let(:pull_request) { nil } + + it 'does not create an external pull request pipeline' do + expect(pipeline).not_to be_persisted + + expect(pipeline.errors[:base]) + .to eq(['Failed to build the pipeline!']) + end + end + end + end + end + describe 'Pipelines for merge requests' do let(:pipeline) do execute_service(source: source, diff --git a/spec/services/clusters/applications/create_service_spec.rb b/spec/services/clusters/applications/create_service_spec.rb index bb86a742f0e..8dd573c3698 100644 --- a/spec/services/clusters/applications/create_service_spec.rb +++ b/spec/services/clusters/applications/create_service_spec.rb @@ -147,12 +147,12 @@ describe Clusters::Applications::CreateService do using RSpec::Parameterized::TableSyntax - where(:application, :association, :allowed, :pre_create_helm) do - 'helm' | :application_helm | true | false - 'ingress' | :application_ingress | true | true - 'runner' | :application_runner | true | true - 'prometheus' | :application_prometheus | true | true - 'jupyter' | :application_jupyter | false | true + where(:application, :association, :allowed, :pre_create_helm, :pre_create_ingress) do + 'helm' | :application_helm | true | false | false + 'ingress' | :application_ingress | true | true | false + 'runner' | :application_runner | true | true | false + 'prometheus' | :application_prometheus | true | true | false + 'jupyter' | :application_jupyter | true | true | true end with_them do @@ -160,6 +160,7 @@ describe Clusters::Applications::CreateService do klass = "Clusters::Applications::#{application.titleize}" allow_any_instance_of(klass.constantize).to receive(:make_scheduled!).and_call_original create(:clusters_applications_helm, :installed, cluster: cluster) if pre_create_helm + create(:clusters_applications_ingress, :installed, cluster: cluster, external_hostname: 'example.com') if pre_create_ingress end let(:params) { { application: application } } diff --git a/spec/services/external_pull_requests/create_pipeline_service_spec.rb b/spec/services/external_pull_requests/create_pipeline_service_spec.rb new file mode 100644 index 00000000000..a4da5b38b97 --- /dev/null +++ b/spec/services/external_pull_requests/create_pipeline_service_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ExternalPullRequests::CreatePipelineService do + describe '#execute' do + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + let(:pull_request) { create(:external_pull_request, project: project) } + + before do + project.add_maintainer(user) + end + + subject { described_class.new(project, user).execute(pull_request) } + + context 'when pull request is open' do + before do + pull_request.update!(status: :open) + end + + context 'when source sha is the head of the source branch' do + let(:source_branch) { project.repository.branches.last } + let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) } + + before do + pull_request.update!(source_branch: source_branch.name, source_sha: source_branch.target) + end + + it 'creates a pipeline for external pull request' do + expect(subject).to be_valid + expect(subject).to be_persisted + expect(subject).to be_external_pull_request_event + expect(subject).to eq(project.ci_pipelines.last) + expect(subject.external_pull_request).to eq(pull_request) + expect(subject.user).to eq(user) + expect(subject.status).to eq('pending') + expect(subject.ref).to eq(pull_request.source_branch) + expect(subject.sha).to eq(pull_request.source_sha) + expect(subject.source_sha).to eq(pull_request.source_sha) + end + end + + context 'when source sha is not the head of the source branch (force push upon rebase)' do + let(:source_branch) { project.repository.branches.first } + let(:commit) { project.repository.commits(source_branch.name, limit: 2).last } + + before do + pull_request.update!(source_branch: source_branch.name, source_sha: commit.sha) + end + + it 'does nothing' do + expect(Ci::CreatePipelineService).not_to receive(:new) + + expect(subject).to be_nil + end + end + end + + context 'when pull request is not opened' do + before do + pull_request.update!(status: :closed) + end + + it 'does nothing' do + expect(Ci::CreatePipelineService).not_to receive(:new) + + expect(subject).to be_nil + end + end + end +end diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb index d9e607cd251..c3a4f3dbe3f 100644 --- a/spec/services/git/branch_push_service_spec.rb +++ b/spec/services/git/branch_push_service_spec.rb @@ -99,6 +99,20 @@ describe Git::BranchPushService, services: true do expect(pipeline).to be_push expect(Gitlab::Git::BRANCH_REF_PREFIX + pipeline.ref).to eq(ref) end + + context 'when pipeline has errors' do + before do + config = YAML.dump({ test: { script: 'ls', only: ['feature'] } }) + stub_ci_pipeline_yaml_file(config) + end + + it 'reports an error' do + allow(Sidekiq).to receive(:server?).and_return(true) + expect(Sidekiq.logger).to receive(:warn) + + expect { subject }.not_to change { Ci::Pipeline.count } + end + end end describe "Updates merge requests" do diff --git a/spec/services/merge_requests/push_options_handler_service_spec.rb b/spec/services/merge_requests/push_options_handler_service_spec.rb index a27fea0c90f..ff4cdd3e7e2 100644 --- a/spec/services/merge_requests/push_options_handler_service_spec.rb +++ b/spec/services/merge_requests/push_options_handler_service_spec.rb @@ -13,6 +13,9 @@ describe MergeRequests::PushOptionsHandlerService do let(:target_branch) { 'feature' } let(:title) { 'my title' } let(:description) { 'my description' } + let(:label1) { 'mylabel1' } + let(:label2) { 'mylabel2' } + let(:label3) { 'mylabel3' } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" } @@ -122,6 +125,16 @@ describe MergeRequests::PushOptionsHandlerService do end end + shared_examples_for 'a service that can change labels of a merge request' do |count| + subject(:last_mr) { MergeRequest.last } + + it 'changes label count' do + service.execute + + expect(last_mr.label_ids.count).to eq(count) + end + end + shared_examples_for 'a service that does not create a merge request' do it do expect { service.execute }.not_to change { MergeRequest.count } @@ -504,6 +517,138 @@ describe MergeRequests::PushOptionsHandlerService do end end + describe '`label` push option' do + let(:push_options) { { label: { label1 => 1, label2 => 1 } } } + + context 'with a new branch' do + let(:changes) { new_branch_changes } + + it_behaves_like 'a service that does not create a merge request' + + it 'adds an error to the service' do + error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}" + + service.execute + + expect(service.errors).to include(error) + end + + context 'when coupled with the `create` push option' do + let(:push_options) { { create: true, label: { label1 => 1, label2 => 1 } } } + + it_behaves_like 'a service that can create a merge request' + it_behaves_like 'a service that can change labels of a merge request', 2 + end + end + + context 'with an existing branch but no open MR' do + let(:changes) { existing_branch_changes } + + it_behaves_like 'a service that does not create a merge request' + + it 'adds an error to the service' do + error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}" + + service.execute + + expect(service.errors).to include(error) + end + + context 'when coupled with the `create` push option' do + let(:push_options) { { create: true, label: { label1 => 1, label2 => 1 } } } + + it_behaves_like 'a service that can create a merge request' + it_behaves_like 'a service that can change labels of a merge request', 2 + end + end + + context 'with an existing branch that has a merge request open' do + let(:changes) { existing_branch_changes } + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch)} + + it_behaves_like 'a service that does not create a merge request' + it_behaves_like 'a service that can change labels of a merge request', 2 + end + + context 'with a deleted branch' do + let(:changes) { deleted_branch_changes } + + it_behaves_like 'a service that does nothing' + end + + context 'with the project default branch' do + let(:changes) { default_branch_changes } + + it_behaves_like 'a service that does nothing' + end + end + + describe '`unlabel` push option' do + let(:push_options) { { label: { label1 => 1, label2 => 1 }, unlabel: { label1 => 1, label3 => 1 } } } + + context 'with a new branch' do + let(:changes) { new_branch_changes } + + it_behaves_like 'a service that does not create a merge request' + + it 'adds an error to the service' do + error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}" + + service.execute + + expect(service.errors).to include(error) + end + + context 'when coupled with the `create` push option' do + let(:push_options) { { create: true, label: { label1 => 1, label2 => 1 }, unlabel: { label1 => 1, label3 => 1 } } } + + it_behaves_like 'a service that can create a merge request' + it_behaves_like 'a service that can change labels of a merge request', 1 + end + end + + context 'with an existing branch but no open MR' do + let(:changes) { existing_branch_changes } + + it_behaves_like 'a service that does not create a merge request' + + it 'adds an error to the service' do + error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}" + + service.execute + + expect(service.errors).to include(error) + end + + context 'when coupled with the `create` push option' do + let(:push_options) { { create: true, label: { label1 => 1, label2 => 1 }, unlabel: { label1 => 1, label3 => 1 } } } + + it_behaves_like 'a service that can create a merge request' + it_behaves_like 'a service that can change labels of a merge request', 1 + end + end + + context 'with an existing branch that has a merge request open' do + let(:changes) { existing_branch_changes } + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch)} + + it_behaves_like 'a service that does not create a merge request' + it_behaves_like 'a service that can change labels of a merge request', 1 + end + + context 'with a deleted branch' do + let(:changes) { deleted_branch_changes } + + it_behaves_like 'a service that does nothing' + end + + context 'with the project default branch' do + let(:changes) { default_branch_changes } + + it_behaves_like 'a service that does nothing' + end + end + describe 'multiple pushed branches' do let(:push_options) { { create: true } } let(:changes) do diff --git a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb new file mode 100644 index 00000000000..afc6f59b773 --- /dev/null +++ b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb @@ -0,0 +1,221 @@ +# frozen_string_literal: true +# +# Requires let variables: +# * throttle_setting_prefix: "throttle_authenticated_api", "throttle_authenticated_web", "throttle_protected_paths" +# * get_args +# * other_user_get_args +# * requests_per_period +# * period_in_seconds +# * period +shared_examples_for 'rate-limited token-authenticated requests' do + before do + # Set low limits + settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period + settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds + end + + context 'when the throttle is enabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the rate limit' do + # At first, allow requests under the rate limit. + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + # the last straw + expect_rejection { get(*get_args) } + end + + it 'allows requests after throttling and then waiting for the next period' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + expect_rejection { get(*get_args) } + + Timecop.travel(period.from_now) do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + expect_rejection { get(*get_args) } + end + end + + it 'counts requests from different users separately, even from the same IP' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + # would be over the limit if this wasn't a different user + get(*other_user_get_args) + expect(response).to have_http_status 200 + end + + it 'counts all requests from the same user, even via different IPs' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') + + expect_rejection { get(*get_args) } + end + + it 'logs RackAttack info into structured logs' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + arguments = { + message: 'Rack_Attack', + env: :throttle, + remote_ip: '127.0.0.1', + request_method: 'GET', + path: get_args.first, + user_id: user.id, + username: user.username + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once + + expect_rejection { get(*get_args) } + end + end + + context 'when the throttle is disabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = false + stub_application_setting(settings_to_set) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + get(*get_args) + expect(response).to have_http_status 200 + end + end + end +end + +# Requires let variables: +# * throttle_setting_prefix: "throttle_authenticated_web" or "throttle_protected_paths" +# * user +# * url_that_requires_authentication +# * requests_per_period +# * period_in_seconds +# * period +shared_examples_for 'rate-limited web authenticated requests' do + before do + login_as(user) + + # Set low limits + settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period + settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds + end + + context 'when the throttle is enabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the rate limit' do + # At first, allow requests under the rate limit. + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + # the last straw + expect_rejection { get url_that_requires_authentication } + end + + it 'allows requests after throttling and then waiting for the next period' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_requires_authentication } + + Timecop.travel(period.from_now) do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_requires_authentication } + end + end + + it 'counts requests from different users separately, even from the same IP' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + # would be over the limit if this wasn't a different user + login_as(create(:user)) + + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + it 'counts all requests from the same user, even via different IPs' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') + + expect_rejection { get url_that_requires_authentication } + end + + it 'logs RackAttack info into structured logs' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + arguments = { + message: 'Rack_Attack', + env: :throttle, + remote_ip: '127.0.0.1', + request_method: 'GET', + path: '/dashboard/snippets', + user_id: user.id, + username: user.username + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once + + get url_that_requires_authentication + end + end + + context 'when the throttle is disabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = false + stub_application_setting(settings_to_set) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + end + end +end diff --git a/spec/workers/update_external_pull_requests_worker_spec.rb b/spec/workers/update_external_pull_requests_worker_spec.rb new file mode 100644 index 00000000000..f3956bb3514 --- /dev/null +++ b/spec/workers/update_external_pull_requests_worker_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe UpdateExternalPullRequestsWorker do + describe '#perform' do + set(:project) { create(:project, import_source: 'tanuki/repository') } + set(:user) { create(:user) } + let(:worker) { described_class.new } + + before do + create(:external_pull_request, + project: project, + source_repository: project.import_source, + target_repository: project.import_source, + source_branch: 'feature-1', + target_branch: 'master') + + create(:external_pull_request, + project: project, + source_repository: project.import_source, + target_repository: project.import_source, + source_branch: 'feature-1', + target_branch: 'develop') + end + + subject { worker.perform(project.id, user.id, ref) } + + context 'when ref is a branch' do + let(:ref) { 'refs/heads/feature-1' } + let(:create_pipeline_service) { instance_double(ExternalPullRequests::CreatePipelineService) } + + it 'runs CreatePipelineService for each pull request matching the source branch and repository' do + expect(ExternalPullRequests::CreatePipelineService) + .to receive(:new) + .and_return(create_pipeline_service) + .twice + expect(create_pipeline_service).to receive(:execute).twice + + subject + end + end + + context 'when ref is not a branch' do + let(:ref) { 'refs/tags/v1.2.3' } + + it 'does nothing' do + expect(ExternalPullRequests::CreatePipelineService).not_to receive(:new) + + subject + end + end + end +end diff --git a/tmp/.gitignore b/tmp/.gitignore new file mode 100644 index 00000000000..a383b23fb78 --- /dev/null +++ b/tmp/.gitignore @@ -0,0 +1,5 @@ +* +!*/ +!.gitignore +!.gitkeep +/tests/ diff --git a/yarn.lock b/yarn.lock index 92da409f544..88f5b82b283 100644 --- a/yarn.lock +++ b/yarn.lock @@ -991,15 +991,15 @@ dependencies: vue-eslint-parser "^6.0.4" -"@gitlab/svgs@^1.71.0": - version "1.71.0" - resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.71.0.tgz#c8e6e8f500ea91e5cbba4ac08df533fb2e622a00" - integrity sha512-kkeNic/FFwaqKnzwio4NE7whBOZ/toRJ8cS0587DBotajAzSYhph5ij4TCY2GTjPa33zIJ5OUr/k90C0Kr71hQ== - -"@gitlab/ui@5.21.0": - version "5.21.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.21.0.tgz#975cf0bca3d16dd080d67ed392b9d24cd64695ac" - integrity sha512-8TMVM+pJXf7omHgKMMZ1FiltuyMOTwfQ3iFgorQzcuhio9u35DJpWi45S2TF7m6CrlpJi7dMX3BsXLbF7ViSUw== +"@gitlab/svgs@^1.72.0": + version "1.72.0" + resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.72.0.tgz#78a29fd383a5a2b31ef91670068a6fea05ba234e" + integrity sha512-EVKRqrXsCY6tUiVAh+lpFMJAyNXZwfEqv5NeKH5ginhALMlOunRkY5rsDllyNvgZ0DWHJS1KEKJj2oVU1ouwAg== + +"@gitlab/ui@5.21.1": + version "5.21.1" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.21.1.tgz#8215ab3eae4296845596d5b3a987d5460b030569" + integrity sha512-TjPVhex9sQGUVwebaJK5XuopDHWx4+Sh9N7yH5u8eXSFWa8vk11voR4qYVt7DZB7powAO/+iiXxYMLLNtXmC/g== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.2.1" |
