diff options
124 files changed, 2415 insertions, 1133 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/.gitlab/issue_templates/Feature proposal.md b/.gitlab/issue_templates/Feature proposal.md index 2ba6b68a53e..5d93758595a 100644 --- a/.gitlab/issue_templates/Feature proposal.md +++ b/.gitlab/issue_templates/Feature proposal.md @@ -17,12 +17,13 @@ Personas can be found at https://about.gitlab.com/handbook/marketing/product-mar ### Permissions and Security -<!-- What permissions are required to perform the described actions? Are they consistent with the existing permissions as documented for users, groups, and projects as appropriate? Is the proposed behavior consistent between the UI, API, and other access methods (e.g. email replies)? --> +<!-- What permissions are required to perform the described actions? Are they consistent with the existing permissions as documented for users, groups, and projects as appropriate? Is the proposed behavior consistent between the UI, API, and other access methods (e.g. email replies)?--> ### Documentation <!-- See the Feature Change Documentation Workflow https://docs.gitlab.com/ee/development/documentation/feature-change-workflow.html -Add all known Documentation Requirements here, per https://docs.gitlab.com/ee/development/documentation/feature-change-workflow.html#documentation-requirements --> +Add all known Documentation Requirements here, per https://docs.gitlab.com/ee/development/documentation/feature-change-workflow.html#documentation-requirements +If this feature requires changing permissions, this document https://docs.gitlab.com/ee/user/permissions.html must be updated accordingly. --> ### Testing 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/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue index 970f5a7b297..b6da572b201 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" 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/lib/utils/axios_utils.js b/app/assets/javascripts/lib/utils/axios_utils.js index 69159e2d741..37721cd030c 100644 --- a/app/assets/javascripts/lib/utils/axios_utils.js +++ b/app/assets/javascripts/lib/utils/axios_utils.js @@ -10,21 +10,18 @@ axios.defaults.headers.common['X-Requested-With'] = 'XMLHttpRequest'; axios.interceptors.request.use(config => { window.activeVueResources = window.activeVueResources || 0; window.activeVueResources += 1; - return config; }); // Remove the global counter axios.interceptors.response.use( - config => { + response => { window.activeVueResources -= 1; - - return config; + return response; }, - e => { + err => { window.activeVueResources -= 1; - - return Promise.reject(e); + return Promise.reject(err); }, ); 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/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/external_link_helper.rb b/app/helpers/external_link_helper.rb new file mode 100644 index 00000000000..9dbad1f5032 --- /dev/null +++ b/app/helpers/external_link_helper.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module ExternalLinkHelper + def external_link(body, url, options = {}) + link_to url, { target: '_blank', rel: 'noopener noreferrer' }.merge(options) do + "#{body} #{icon('external-link')}".html_safe + 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/build.rb b/app/models/ci/build.rb index d558f66154e..72782827906 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -88,6 +88,7 @@ module Ci validates :coverage, numericality: true, allow_blank: true validates :ref, presence: true + scope :not_interruptible, -> { joins(:metadata).where(ci_builds_metadata: { interruptible: false }) } scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } scope :with_artifacts_archive, ->() do diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 2b6f10ef79f..d620959b538 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -225,6 +225,14 @@ module Ci where('EXISTS (?)', ::Ci::Build.latest.with_reports(reports_scope).where('ci_pipelines.id=ci_builds.commit_id').select(1)) end + scope :without_interruptible_builds, -> do + where('NOT EXISTS (?)', + Ci::Build.where('ci_builds.commit_id = ci_pipelines.id') + .with_status(:running, :success, :failed) + .not_interruptible + ) + end + # Returns the pipelines in descending order (= newest first), optionally # limited to a number of references. # 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..ef1af1fc8bc 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 }.merge(PROJECT_ONLY_APPLICATIONS).freeze DEFAULT_ENVIRONMENT = '*' KUBE_INGRESS_BASE_DOMAIN = 'KUBE_INGRESS_BASE_DOMAIN' diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index 304cc71e9dc..a0ca8a34c6d 100644 --- a/app/models/concerns/ci/metadatable.rb +++ b/app/models/concerns/ci/metadatable.rb @@ -15,6 +15,7 @@ module Ci autosave: true delegate :timeout, to: :metadata, prefix: true, allow_nil: true + delegate :interruptible, to: :metadata, prefix: false, allow_nil: true before_create :ensure_metadata end @@ -50,6 +51,14 @@ module Ci write_metadata_attribute(:yaml_variables, :config_variables, value) end + def interruptible + metadata&.interruptible + end + + def interruptible=(value) + ensure_metadata.interruptible = value + end + private def read_metadata_attribute(legacy_key, metadata_key, default_value = nil) diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index cf88076ac74..bcbbb27a9a8 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -102,6 +102,7 @@ module HasStatus scope :manual, -> { with_status(:manual) } scope :scheduled, -> { with_status(:scheduled) } scope :alive, -> { with_status(:created, :preparing, :pending, :running) } + scope :alive_or_scheduled, -> { with_status(:created, :preparing, :pending, :running, :scheduled) } scope :created_or_pending, -> { with_status(:created, :pending) } scope :running_or_pending, -> { with_status(:running, :pending) } scope :finished, -> { with_status(:success, :failed, :canceled) } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 28e450f9b30..95daa48d4bc 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -174,6 +174,7 @@ class MergeRequest < ApplicationRecord scope :from_project, ->(project) { where(source_project_id: project.id) } scope :merged, -> { with_state(:merged) } scope :closed_and_merged, -> { with_states(:closed, :merged) } + scope :open_and_closed, -> { with_states(:opened, :closed) } scope :from_source_branches, ->(branches) { where(source_branch: branches) } scope :by_commit_sha, ->(sha) do where('EXISTS (?)', MergeRequestDiff.select(1).where('merge_requests.latest_merge_request_diff_id = merge_request_diffs.id').by_commit_sha(sha)).reorder(nil) @@ -187,6 +188,11 @@ class MergeRequest < ApplicationRecord target_project: [:route, { namespace: :route }], source_project: [:route, { namespace: :route }]) } + scope :by_target_branch_wildcard, ->(wildcard_branch_name) do + where("target_branch LIKE ?", ApplicationRecord.sanitize_sql_like(wildcard_branch_name).tr('*', '%')) + end + scope :by_target_branch, ->(branch_name) { where(target_branch: branch_name) } + scope :preload_source_project, -> { preload(:source_project) } after_save :keep_around_commit @@ -1232,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/protected_branch.rb b/app/models/protected_branch.rb index ee0c94c20af..9fd929371f8 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -3,6 +3,9 @@ class ProtectedBranch < ApplicationRecord include ProtectedRef + scope :requiring_code_owner_approval, + -> { where(code_owner_approval_required: true) } + protected_ref_access_levels :merge, :push def self.protected_ref_accessible_to?(ref, user, project:, action:, protected_refs: nil) 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 29317f1176e..8f8582afb43 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -91,11 +91,21 @@ module Ci # rubocop: disable CodeReuse/ActiveRecord def auto_cancelable_pipelines - project.ci_pipelines - .where(ref: pipeline.ref) - .where.not(id: pipeline.id) - .where.not(sha: project.commit(pipeline.ref).try(:id)) - .created_or_pending + # TODO: Introduced by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/23464 + if Feature.enabled?(:ci_support_interruptible_pipelines, project, default_enabled: true) + project.ci_pipelines + .where(ref: pipeline.ref) + .where.not(id: pipeline.id) + .where.not(sha: project.commit(pipeline.ref).try(:id)) + .alive_or_scheduled + .without_interruptible_builds + else + project.ci_pipelines + .where(ref: pipeline.ref) + .where.not(id: pipeline.id) + .where.not(sha: project.commit(pipeline.ref).try(:id)) + .created_or_pending + end end # rubocop: enable CodeReuse/ActiveRecord 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/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb index 87aaf4672a4..6b2836bba39 100644 --- a/app/services/protected_branches/create_service.rb +++ b/app/services/protected_branches/create_service.rb @@ -5,7 +5,8 @@ module ProtectedBranches def execute(skip_authorization: false) raise Gitlab::Access::AccessDeniedError unless skip_authorization || authorized? - protected_branch.save + save_protected_branch + protected_branch end @@ -15,6 +16,10 @@ module ProtectedBranches private + def save_protected_branch + protected_branch.save + end + def protected_branch @protected_branch ||= project.protected_branches.new(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/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/projects/pages/_access.html.haml b/app/views/projects/pages/_access.html.haml index 539f223ca9b..7b6d46964a2 100644 --- a/app/views/projects/pages/_access.html.haml +++ b/app/views/projects/pages/_access.html.haml @@ -7,9 +7,11 @@ %strong = _("Your pages are served under:") - %p= link_to @project.pages_url, @project.pages_url + %p + = external_link(@project.pages_url, @project.pages_url) - @project.pages_domains.each do |domain| - %p= link_to domain.url, domain.url + %p + = external_link(domain.url, domain.url) .card-footer.alert-primary = _("It may take up to 30 minutes before the site is available after the first deployment.") diff --git a/app/views/projects/pages/_list.html.haml b/app/views/projects/pages/_list.html.haml index 2427b4d7611..c4285e7f3d2 100644 --- a/app/views/projects/pages/_list.html.haml +++ b/app/views/projects/pages/_list.html.haml @@ -12,9 +12,7 @@ .domain-status.ci-status-icon.has-tooltip{ class: "ci-status-icon-#{status}", title: tooltip } = sprite_icon("status_#{status}", size: 16 ) .domain-name - = link_to domain.url do - = domain.url - = icon('external-link') + = external_link(domain.url, domain.url) - if domain.subject %div %span.badge.badge-gray Certificate: #{domain.subject} diff --git a/app/views/projects/pages_domains/show.html.haml b/app/views/projects/pages_domains/show.html.haml index d0b54946f7e..33837e21c8d 100644 --- a/app/views/projects/pages_domains/show.html.haml +++ b/app/views/projects/pages_domains/show.html.haml @@ -21,9 +21,7 @@ %td = _("Domain") %td - = link_to @domain.url do - = @domain.url - = icon('external-link') + = external_link(@domain.url, @domain.url) %tr %td = _("DNS") 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/changelogs/unreleased/66067-pages-domain-doesnt-set-target-blank.yml b/changelogs/unreleased/66067-pages-domain-doesnt-set-target-blank.yml new file mode 100644 index 00000000000..726d4b163d2 --- /dev/null +++ b/changelogs/unreleased/66067-pages-domain-doesnt-set-target-blank.yml @@ -0,0 +1,5 @@ +--- +title: Makes custom Pages domain open as external link in new tab +merge_request: 32130 +author: jakeburden +type: fixed diff --git a/changelogs/unreleased/66150-remove-dynamically-constructed-feature-flags-starting-with-promethe.yml b/changelogs/unreleased/66150-remove-dynamically-constructed-feature-flags-starting-with-promethe.yml new file mode 100644 index 00000000000..e6dc1f1bec6 --- /dev/null +++ b/changelogs/unreleased/66150-remove-dynamically-constructed-feature-flags-starting-with-promethe.yml @@ -0,0 +1,5 @@ +--- +title: Remove dynamically constructed feature flags starting with prometheus_transaction_ +merge_request: 32395 +author: Jacopo Beschi @jacopo-beschi +type: changed 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/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/issue-32741.yml b/changelogs/unreleased/issue-32741.yml new file mode 100644 index 00000000000..5d0a3ee2cc7 --- /dev/null +++ b/changelogs/unreleased/issue-32741.yml @@ -0,0 +1,5 @@ +--- +title: New interruptible attribute for CI/CD jobs +merge_request: 23464 +author: Cédric Tabin +type: added 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/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/20190729180447_add_merge_requests_require_code_owner_approval_to_protected_branches.rb b/db/migrate/20190729180447_add_merge_requests_require_code_owner_approval_to_protected_branches.rb new file mode 100644 index 00000000000..098fcff9ace --- /dev/null +++ b/db/migrate/20190729180447_add_merge_requests_require_code_owner_approval_to_protected_branches.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class AddMergeRequestsRequireCodeOwnerApprovalToProtectedBranches < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default( + :protected_branches, + :code_owner_approval_required, + :boolean, + default: false + ) + + add_concurrent_index( + :protected_branches, + [:project_id, :code_owner_approval_required], + name: "code_owner_approval_required", + where: "code_owner_approval_required = #{Gitlab::Database.true_value}") + end + + def down + remove_concurrent_index(:protected_branches, name: "code_owner_approval_required") + + remove_column(:protected_branches, :code_owner_approval_required) + end +end diff --git a/db/migrate/20190905223800_add_interruptible_to_builds_metadata.rb b/db/migrate/20190905223800_add_interruptible_to_builds_metadata.rb new file mode 100644 index 00000000000..a666b11013b --- /dev/null +++ b/db/migrate/20190905223800_add_interruptible_to_builds_metadata.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddInterruptibleToBuildsMetadata < ActiveRecord::Migration[5.0] + DOWNTIME = false + + def change + add_column :ci_builds_metadata, :interruptible, :boolean + end +end diff --git a/db/migrate/20190905223900_add_concurrent_index_to_builds_metadata.rb b/db/migrate/20190905223900_add_concurrent_index_to_builds_metadata.rb new file mode 100644 index 00000000000..d394f995673 --- /dev/null +++ b/db/migrate/20190905223900_add_concurrent_index_to_builds_metadata.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddConcurrentIndexToBuildsMetadata < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_builds_metadata, [:build_id], + where: "interruptible = false", + name: "index_ci_builds_metadata_on_build_id_and_interruptible_false" + end + + def down + remove_concurrent_index_by_name(:ci_builds_metadata, 'index_ci_builds_metadata_on_build_id_and_interruptible_false') + end +end diff --git a/db/schema.rb b/db/schema.rb index 98c4403efe1..61f7787f192 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_09_04_173203) do +ActiveRecord::Schema.define(version: 2019_09_05_223900) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -619,9 +619,11 @@ ActiveRecord::Schema.define(version: 2019_09_04_173203) do t.integer "project_id", null: false t.integer "timeout" t.integer "timeout_source", default: 1, null: false + t.boolean "interruptible" t.jsonb "config_options" t.jsonb "config_variables" t.index ["build_id"], name: "index_ci_builds_metadata_on_build_id", unique: true + t.index ["build_id"], name: "index_ci_builds_metadata_on_build_id_and_interruptible_false", where: "(interruptible = false)" t.index ["project_id"], name: "index_ci_builds_metadata_on_project_id" end @@ -2932,6 +2934,8 @@ ActiveRecord::Schema.define(version: 2019_09_04_173203) do t.string "name", null: false t.datetime "created_at" t.datetime "updated_at" + t.boolean "code_owner_approval_required", default: false, null: false + t.index ["project_id", "code_owner_approval_required"], name: "code_owner_approval_required", where: "(code_owner_approval_required = true)" t.index ["project_id"], name: "index_protected_branches_on_project_id" end diff --git a/doc/administration/database_load_balancing.md b/doc/administration/database_load_balancing.md index 3b1151ae570..f643d853d10 100644 --- a/doc/administration/database_load_balancing.md +++ b/doc/administration/database_load_balancing.md @@ -216,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/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index ca48326c6d0..6dbfd5404d0 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -91,33 +91,6 @@ The following metrics can be controlled by feature flags: |:---------------------------------------------------------------|:-------------------------------------------------------------------| | `gitlab_method_call_duration_seconds` | `prometheus_metrics_method_instrumentation` | | `gitlab_transaction_allocated_memory_bytes` | `prometheus_metrics_transaction_allocated_memory` | -| `gitlab_transaction_event_build_found_total` | `prometheus_transaction_event_build_found_total` | -| `gitlab_transaction_event_build_invalid_total` | `prometheus_transaction_event_build_invalid_total` | -| `gitlab_transaction_event_build_not_found_cached_total` | `prometheus_transaction_event_build_not_found_cached_total` | -| `gitlab_transaction_event_build_not_found_total` | `prometheus_transaction_event_build_not_found_total` | -| `gitlab_transaction_event_change_default_branch_total` | `prometheus_transaction_event_change_default_branch_total` | -| `gitlab_transaction_event_create_repository_total` | `prometheus_transaction_event_create_repository_total` | -| `gitlab_transaction_event_etag_caching_cache_hit_total` | `prometheus_transaction_event_etag_caching_cache_hit_total` | -| `gitlab_transaction_event_etag_caching_header_missing_total` | `prometheus_transaction_event_etag_caching_header_missing_total` | -| `gitlab_transaction_event_etag_caching_key_not_found_total` | `prometheus_transaction_event_etag_caching_key_not_found_total` | -| `gitlab_transaction_event_etag_caching_middleware_used_total` | `prometheus_transaction_event_etag_caching_middleware_used_total` | -| `gitlab_transaction_event_etag_caching_resource_changed_total` | `prometheus_transaction_event_etag_caching_resource_changed_total` | -| `gitlab_transaction_event_fork_repository_total` | `prometheus_transaction_event_fork_repository_total` | -| `gitlab_transaction_event_import_repository_total` | `prometheus_transaction_event_import_repository_total` | -| `gitlab_transaction_event_push_branch_total` | `prometheus_transaction_event_push_branch_total` | -| `gitlab_transaction_event_push_commit_total` | `prometheus_transaction_event_push_commit_total` | -| `gitlab_transaction_event_push_tag_total` | `prometheus_transaction_event_push_tag_total` | -| `gitlab_transaction_event_rails_exception_total` | `prometheus_transaction_event_rails_exception_total` | -| `gitlab_transaction_event_receive_email_total` | `prometheus_transaction_event_receive_email_total` | -| `gitlab_transaction_event_remote_mirrors_failed_total` | `prometheus_transaction_event_remote_mirrors_failed_total` | -| `gitlab_transaction_event_remote_mirrors_finished_total` | `prometheus_transaction_event_remote_mirrors_finished_total` | -| `gitlab_transaction_event_remote_mirrors_running_total` | `prometheus_transaction_event_remote_mirrors_running_total` | -| `gitlab_transaction_event_remove_branch_total` | `prometheus_transaction_event_remove_branch_total` | -| `gitlab_transaction_event_remove_repository_total` | `prometheus_transaction_event_remove_repository_total` | -| `gitlab_transaction_event_remove_tag_total` | `prometheus_transaction_event_remove_tag_total` | -| `gitlab_transaction_event_sidekiq_exception_total` | `prometheus_transaction_event_sidekiq_exception_total` | -| `gitlab_transaction_event_stuck_import_jobs_total` | `prometheus_transaction_event_stuck_import_jobs_total` | -| `gitlab_transaction_event_update_build_total` | `prometheus_transaction_event_update_build_total` | | `gitlab_view_rendering_duration_seconds` | `prometheus_metrics_view_instrumentation` | ## Sidekiq Metrics available for Geo **(PREMIUM)** 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/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/ci/yaml/README.md b/doc/ci/yaml/README.md index bfba21646b5..8f2e95dbb10 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -116,6 +116,7 @@ The following table lists available parameters for jobs: | [`extends`](#extends) | Configuration entries that this job is going to inherit from. | | [`pages`](#pages) | Upload the result of a job to use with GitLab Pages. | | [`variables`](#variables) | Define job variables on a job level. | +| [interruptible](#interruptible) | Defines if a job can be canceled when made redundant by a newer run | NOTE: **Note:** Parameters `types` and `type` are [deprecated](#deprecated-parameters). @@ -2083,6 +2084,46 @@ staging: branch: stable ``` +### `interruptible` + +`interruptible` is used to indicate that a job should be canceled if made redundant by a newer run of the same job. Defaults to `false` if there is an environment defined and `true` otherwise. +This value will only be used if the [automatic cancellation of redundant pipelines feature](https://docs.gitlab.com/ee/user/project/pipelines/settings.html#auto-cancel-pending-pipelines) +is enabled. + +When enabled, a pipeline on the same branch will be canceled when: + +- It is made redundant by a newer pipeline run. +- Either all jobs are set as interruptible, or any uninterruptible jobs are not yet pending. + +Pending jobs are always considered interruptible. + +TIP: **Tip:** +Set jobs as uninterruptible that should behave atomically and should never be canceled once started. + +Here is a simple example: + +```yaml +stages: + - stage1 + - stage2 + +step-1: + stage: stage1 + script: + - echo "Can be canceled" + +step-2: + stage: stage2 + script: + - echo "Can not be canceled" + interruptible: false +``` + +In the example above, a new pipeline run will cause an existing running pipeline to be: + +- Canceled, if only `step-1` is running or pending. +- Not canceled, once `step-2` becomes pending. + ### `include` > - Introduced in [GitLab Premium](https://about.gitlab.com/pricing/) 10.5. 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/documentation/index.md b/doc/development/documentation/index.md index 719b9aa212a..c267669aa79 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -463,7 +463,7 @@ The current tests are: to [check locally](#previewing-the-changes-live) before pushing to GitLab by executing the command `bundle exec nanoc check internal_links` on your local [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs) directory. In addition, - `docs-lint` also runs [markdownlint](styleguide.md#markdown-rules) to ensure the + `docs-lint` also runs [`markdownlint`](#markdownlint) to ensure the markdown is consistent across all documentation. 1. [`ee_compat_check`](../automatic_ce_ee_merge.md#avoiding-ce-ee-merge-conflicts-beforehand) (runs on CE only): When you submit a merge request to GitLab Community Edition (CE), @@ -488,7 +488,7 @@ help you catch common issues before raising merge requests for review of documen The following are some suggested linters you can install locally and sample configuration: - [`proselint`](#proselint) -- [`markdownlint`](#markdownlint) +- [`markdownlint`](#markdownlint), which is the same as the test run in [`docs-lint`](#testing) NOTE: **Note:** This list does not limit what other linters you can add to your local documentation writing toolchain. @@ -534,76 +534,56 @@ A file with `proselint` configuration must be placed in a #### `markdownlint` -`markdownlint` checks that certain rules ([example](https://github.com/DavidAnson/markdownlint/blob/master/README.md#rules--aliases)) -are followed for Markdown syntax. Our [Documentation Style Guide](styleguide.md) and -[Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) +[`markdownlint`](https://github.com/DavidAnson/markdownlint) checks that markdown +syntax follows [certain rules](https://github.com/DavidAnson/markdownlint/blob/master/doc/Rules.md#rules), +and is used by the [`docs-lint` test](#testing) with a [configuration file](#markdownlint-configuration). +Our [Documentation Style Guide](styleguide.md#markdown) and [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) elaborate on which choices must be made when selecting Markdown syntax for GitLab -documentation. This tool helps catch deviations from those guidelines, and matches the -tests run on the documentation by [`docs-lint`](#testing). +documentation. This tool helps catch deviations from those guidelines. `markdownlint` can be used [on the command line](https://github.com/igorshubovych/markdownlint-cli#markdownlint-cli--), either on a single Markdown file or on all Markdown files in a project. For example, to run `markdownlint` on all documentation in the [`gitlab-ce` project](https://gitlab.com/gitlab-org/gitlab-ce), -run the following commands from within the `gitlab-ce` project: +run the following commands from within your `gitlab-ce` project root directory, which will +automatically detect the [`.markdownlint.json`](#markdownlint-configuration) config +file in the root of the project, and test all files in `/doc` and its subdirectories: ```sh -cd doc -markdownlint **/*.md +markdownlint 'doc/**/*.md' ``` -`markdownlint` can also be run from within editors using plugins. For example, the following plugins - are available: +If you wish to use a different config file, use the `-c` flag: + +```sh +markdownlint -c <config-file-name> 'doc/**/*.md' +``` + +`markdownlint` can also be run from within text editors using [plugins/extensions](https://github.com/DavidAnson/markdownlint#related), +such as: - [Sublime Text](https://packagecontrol.io/packages/SublimeLinter-contrib-markdownlint) - [Visual Studio Code](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) -- [Others](https://github.com/DavidAnson/markdownlint#related) +- [Atom](https://atom.io/packages/linter-node-markdownlint) -##### Sample `markdownlint` configuration +It is best to use the [same configuration file](#markdownlint-configuration) as what +is in use in the four repos that are the sources for <https://docs.gitlab.com>. Each +plugin/extension has different requirements regarding the configuration file, which +is explained in each editor's docs. -The following sample `markdownlint` configuration modifies the available default rules to: +##### `markdownlint` configuration -- Adhere to the [Documentation Style Guide](styleguide.md). -- Apply conventions found in the GitLab documentation. -- Allow the flexibility of using some inline HTML. +Each formatting issue that `markdownlint` checks has an associated +[rule](https://github.com/DavidAnson/markdownlint/blob/master/doc/Rules.md#rules). +These rules are configured in the `.markdownlint.json` files located in the root of +four repos that are the sources for <https://docs.gitlab.com>: -```json -{ - "default": true, - "header-style": { "style": "atx" }, - "ul-style": { "style": "dash" }, - "line-length": false, - "no-trailing-punctuation": false, - "ol-prefix": { "style": "one" }, - "blanks-around-fences": true, - "no-inline-html": { - "allowed_elements": [ - "table", - "tbody", - "tr", - "td", - "ul", - "ol", - "li", - "br", - "img", - "a", - "strong", - "i", - "div", - "b" - ] - }, - "hr-style": { "style": "---" }, - "code-block-style": { "style": "fenced" }, - "fenced-code-language": false, - "no-duplicate-header": { "allow_different_nesting": true }, - "commands-show-output": false -} -``` +- <https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.markdownlint.json> +- <https://gitlab.com/gitlab-org/gitlab-runner/blob/master/.markdownlint.json> +- <https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/.markdownlint.json> +- <https://gitlab.com/charts/gitlab/blob/master/.markdownlint.json> -For [`markdownlint`](https://github.com/DavidAnson/markdownlint/), this configuration must be -placed in a [valid location](https://github.com/igorshubovych/markdownlint-cli#configuration). For -example, `~/.markdownlintrc`. +By default all rules are enabled, so the configuration file is used to disable unwanted +rules, and also to configure optional parameters for enabled rules as needed. ## Danger Bot diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index 283e8bea8d5..09dd31e2aee 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -110,18 +110,11 @@ Hard-coded HTML is valid, although it's discouraged to be used while we have `/h ### Markdown Rules GitLab ensures that the Markdown used across all documentation is consistent, as -well as easy to review and maintain, by testing documentation changes with -[Markdownlint (mdl)](https://github.com/markdownlint/markdownlint). This lint test -checks many common problems with Markdown, and fails when any document has an issue +well as easy to review and maintain, by [testing documentation changes](index.md#testing) with +[`markdownlint`](index.md#markdownlint). This lint test fails when any document has an issue with Markdown formatting that may cause the page to render incorrectly within GitLab. It will also fail when a document is using non-standard Markdown (which may render -correctly, but is not the current standard in GitLab documentation). - -Each formatting issue that mdl checks has an associated [rule](https://github.com/markdownlint/markdownlint/blob/master/docs/RULES.md), -and the rules that are currently enabled for GitLab documentation are listed in the -[`.mdlrc.style`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.mdlrc.style) file. -Configuration options are set in the [`.mdlrc`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.mdlrc.style) -file. +correctly, but is not the current standard for GitLab documentation). ## Structure @@ -460,7 +453,7 @@ For other punctuation rules, please refer to the This is to ensure that no document with wrong heading is going live without an audit, thus preventing dead links and redirection issues when corrected. -- Leave exactly one new line after a heading. +- Leave exactly one blank line before and after a heading. - Do not use links in headings. - Add the corresponding [product badge](#product-badges) according to the tier the feature belongs. diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index f4cee410066..00371057d3c 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -88,7 +88,7 @@ class RenameUsersUpdatedAtToUpdatedAtTimestamp < ActiveRecord::Migration[4.2] end def down - cleanup_concurrent_column_rename :users, :updated_at_timestamp, :updated_at + undo_rename_column_concurrently :users, :updated_at, :updated_at_timestamp end end ``` @@ -118,7 +118,7 @@ class CleanupUsersUpdatedAtRename < ActiveRecord::Migration[4.2] end def down - rename_column_concurrently :users, :updated_at_timestamp, :updated_at + undo_cleanup_concurrent_column_rename :users, :updated_at, :updated_at_timestamp end end ``` 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/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..83ddcf61664 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 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/index.md b/doc/user/project/merge_requests/index.md index d6da8cb99c7..9f31f38460a 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -289,6 +289,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 +376,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/user/project/protected_branches.md b/doc/user/project/protected_branches.md index 8423b962948..895c8ac88e7 100644 --- a/doc/user/project/protected_branches.md +++ b/doc/user/project/protected_branches.md @@ -15,7 +15,7 @@ By default, a protected branch does four simple things: - It prevents its creation, if not already created, from everybody except users with Maintainer permission. -- It prevents pushes from everybody except users with Maintainer permission. +- It prevents pushes from everybody except users with **Allowed** permission. - It prevents **anyone** from force pushing to the branch. - It prevents **anyone** from deleting the branch. 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/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index 6b438235258..9a80671f996 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -17,6 +17,18 @@ module API @project # rubocop:disable Gitlab/ModuleWithInstanceVariables end + def access_checker_for(actor, protocol) + access_checker_klass.new(actor.key_or_user, project, protocol, + authentication_abilities: ssh_authentication_abilities, + namespace_path: namespace_path, + project_path: project_path, + redirected_path: redirected_path) + end + + def access_checker_klass + repo_type.access_checker_class + end + def ssh_authentication_abilities [ :read_project, diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index 622032b8355..087985d34b0 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -35,36 +35,14 @@ module API # project - project full_path (not path on disk) # action - git action (git-upload-pack or git-receive-pack) # changes - changes as "oldrev newrev ref", see Gitlab::ChangesList - # rubocop: disable CodeReuse/ActiveRecord post "/allowed" do # Stores some Git-specific env thread-safely env = parse_env Gitlab::Git::HookEnv.set(gl_repository, env) if project - actor = - if params[:key_id] - Key.find_by(id: params[:key_id]) - elsif params[:user_id] - User.find_by(id: params[:user_id]) - elsif params[:username] - UserFinder.new(params[:username]).find_by_username - end - - protocol = params[:protocol] - - actor.update_last_used_at if actor.is_a?(Key) - user = - if actor.is_a?(Key) - actor.user - else - actor - end - - access_checker_klass = repo_type.access_checker_class - access_checker = access_checker_klass.new(actor, project, - protocol, authentication_abilities: ssh_authentication_abilities, - namespace_path: namespace_path, project_path: project_path, - redirected_path: redirected_path) + actor = Support::GitAccessActor.from_params(params) + actor.update_last_used_at! + access_checker = access_checker_for(actor, params[:protocol]) check_result = begin result = access_checker.check(params[:action], params[:changes]) @@ -78,15 +56,15 @@ module API break response_with_status(code: 404, success: false, message: e.message) end - log_user_activity(actor) + log_user_activity(actor.user) case check_result when ::Gitlab::GitAccessResult::Success payload = { gl_repository: gl_repository, gl_project_path: gl_project_path, - gl_id: Gitlab::GlId.gl_id(user), - gl_username: user&.username, + gl_id: Gitlab::GlId.gl_id(actor.user), + gl_username: actor.username, git_config_options: [], gitaly: gitaly_payload(params[:action]), gl_console_messages: check_result.console_messages @@ -105,7 +83,6 @@ module API response_with_status(code: 500, success: false, message: UNKNOWN_CHECK_RESULT_ERROR) end end - # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord post "/lfs_authenticate" do diff --git a/lib/api/support/git_access_actor.rb b/lib/api/support/git_access_actor.rb new file mode 100644 index 00000000000..2e0962c6295 --- /dev/null +++ b/lib/api/support/git_access_actor.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module API + module Support + class GitAccessActor + attr_reader :user + + def initialize(user: nil, key: nil) + @user = user + @key = key + + @user = key.user if !user && key + end + + def self.from_params(params) + if params[:key_id] + new(key: Key.find_by_id(params[:key_id])) + elsif params[:user_id] + new(user: UserFinder.new(params[:user_id]).find_by_id) + elsif params[:username] + new(user: UserFinder.new(params[:username]).find_by_username) + end + end + + def key_or_user + key || user + end + + def username + user&.username + end + + def update_last_used_at! + key&.update_last_used_at + end + + private + + attr_reader :key + end + end +end 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/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 6e11c582750..3009c7e8329 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -15,7 +15,7 @@ module Gitlab ALLOWED_KEYS = %i[tags script only except rules type image services allow_failure type stage when start_in artifacts cache dependencies needs before_script after_script variables - environment coverage retry parallel extends].freeze + environment coverage retry parallel extends interruptible].freeze REQUIRED_BY_NEEDS = %i[stage].freeze @@ -37,6 +37,7 @@ module Gitlab with_options allow_nil: true do validates :tags, array_of_strings: true validates :allow_failure, boolean: true + validates :interruptible, boolean: true validates :parallel, numericality: { only_integer: true, greater_than_or_equal_to: 2, less_than_or_equal_to: 50 } @@ -122,10 +123,11 @@ module Gitlab helpers :before_script, :script, :stage, :type, :after_script, :cache, :image, :services, :only, :except, :variables, :artifacts, :environment, :coverage, :retry, - :parallel, :needs + :parallel, :needs, :interruptible attributes :script, :tags, :allow_failure, :when, :dependencies, - :needs, :retry, :parallel, :extends, :start_in, :rules + :needs, :retry, :parallel, :extends, :start_in, :rules, + :interruptible def self.matching?(name, config) !name.to_s.start_with?('.') && @@ -207,6 +209,7 @@ module Gitlab coverage: coverage_defined? ? coverage_value : nil, retry: retry_defined? ? retry_value : nil, parallel: parallel_defined? ? parallel_value.to_i : nil, + interruptible: interruptible_defined? ? interruptible_value : nil, artifacts: artifacts_value, after_script: after_script_value, ignore: ignored?, diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 2e1eab270ff..501d91fa9ad 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -41,6 +41,7 @@ module Gitlab coverage_regex: job[:coverage], yaml_variables: yaml_variables(name), needs_attributes: job[:needs]&.map { |need| { name: need } }, + interruptible: job[:interruptible], options: { image: job[:image], services: job[:services], diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index d30d5a38670..702c73e8e4d 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -38,8 +38,12 @@ module Gitlab ENV['CI_PROJECT_NAME'] == 'gitlab-ee' || File.exist?('../../CHANGELOG-EE.md') end + def gitlab_helper + gitlab if respond_to?(:gitlab) + end + def release_automation? - gitlab.mr_author == RELEASE_TOOLS_BOT + gitlab_helper&.mr_author == RELEASE_TOOLS_BOT end def project_name @@ -83,7 +87,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 +130,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/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 57a413f8e04..5a42952796c 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -459,29 +459,19 @@ module Gitlab check_trigger_permissions!(table) - old_col = column_for(table, old) - new_type = type || old_col.type - - add_column(table, new, new_type, - limit: old_col.limit, - precision: old_col.precision, - scale: old_col.scale) - - # We set the default value _after_ adding the column so we don't end up - # updating any existing data with the default value. This isn't - # necessary since we copy over old values further down. - change_column_default(table, new, old_col.default) unless old_col.default.nil? + create_column_from(table, old, new, type: type) install_rename_triggers(table, old, new) - - update_column_in_batches(table, new, Arel::Table.new(table)[old]) - - change_column_null(table, new, false) unless old_col.null - - copy_indexes(table, old, new) - copy_foreign_keys(table, old, new) end + # Reverses operations performed by rename_column_concurrently. + # + # This method takes care of removing previously installed triggers as well + # as removing the new column. + # + # table - The name of the database table. + # old - The name of the old column. + # new - The name of the new column. def undo_rename_column_concurrently(table, old, new) trigger_name = rename_trigger_name(table, old, new) @@ -557,6 +547,18 @@ module Gitlab remove_column(table, old) end + # Reverses the operations performed by cleanup_concurrent_column_rename. + # + # This method adds back the old_column removed + # by cleanup_concurrent_column_rename. + # It also adds back the (old_column > new_column) trigger that is removed + # by cleanup_concurrent_column_rename. + # + # table - The name of the database table containing the column. + # old - The old column name. + # new - The new column name. + # type - The type of the old column. If no type is given the new column's + # type is used. def undo_cleanup_concurrent_column_rename(table, old, new, type: nil) if transaction_open? raise 'undo_cleanup_concurrent_column_rename can not be run inside a transaction' @@ -564,26 +566,9 @@ module Gitlab check_trigger_permissions!(table) - new_column = column_for(table, new) - - add_column(table, old, type || new_column.type, - limit: new_column.limit, - precision: new_column.precision, - scale: new_column.scale) - - # We set the default value _after_ adding the column so we don't end up - # updating any existing data with the default value. This isn't - # necessary since we copy over old values further down. - change_column_default(table, old, new_column.default) unless new_column.default.nil? + create_column_from(table, new, old, type: type) install_rename_triggers(table, old, new) - - update_column_in_batches(table, old, Arel::Table.new(table)[new]) - - change_column_null(table, old, false) unless new_column.null - - copy_indexes(table, new, old) - copy_foreign_keys(table, new, old) end # Changes the column type of a table using a background migration. @@ -1076,6 +1061,28 @@ into similar problems in the future (e.g. when new tables are created). private + def create_column_from(table, old, new, type: nil) + old_col = column_for(table, old) + new_type = type || old_col.type + + add_column(table, new, new_type, + limit: old_col.limit, + precision: old_col.precision, + scale: old_col.scale) + + # We set the default value _after_ adding the column so we don't end up + # updating any existing data with the default value. This isn't + # necessary since we copy over old values further down. + change_column_default(table, new, old_col.default) unless old_col.default.nil? + + update_column_in_batches(table, new, Arel::Table.new(table)[old]) + + change_column_null(table, new, false) unless old_col.null + + copy_indexes(table, old, new) + copy_foreign_keys(table, old, new) + end + def validate_timestamp_column_name!(column_name) return if PERMITTED_TIMESTAMP_COLUMNS.member?(column_name) diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb index 9584285be7e..ba2a0b2ecf8 100644 --- a/lib/gitlab/metrics/transaction.rb +++ b/lib/gitlab/metrics/transaction.rb @@ -78,7 +78,7 @@ module Gitlab # tags - A set of tags to attach to the event. def add_event(event_name, tags = {}) filtered_tags = filter_tags(tags) - self.class.transaction_metric(event_name, :counter, prefix: 'event_', use_feature_flag: true, tags: filtered_tags).increment(filtered_tags.merge(labels)) + self.class.transaction_metric(event_name, :counter, prefix: 'event_', tags: filtered_tags).increment(filtered_tags.merge(labels)) @metrics << Metric.new(EVENT_SERIES, { count: 1 }, filtered_tags.merge(event: event_name), :event) end @@ -155,12 +155,11 @@ module Gitlab with_feature :prometheus_metrics_transaction_allocated_memory end - def self.transaction_metric(name, type, prefix: nil, use_feature_flag: false, tags: {}) + def self.transaction_metric(name, type, prefix: nil, tags: {}) metric_name = "gitlab_transaction_#{prefix}#{name}_total".to_sym fetch_metric(type, metric_name) do docstring "Transaction #{prefix}#{name} #{type}" base_labels tags.merge(BASE_LABELS) - with_feature "prometheus_transaction_#{prefix}#{name}_total".to_sym if use_feature_flag if type == :gauge multiprocess_mode :livesum 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..c2f5843a9a5 --- /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_relative '../../lib/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/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/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/frontend/clusters/components/applications_spec.js b/spec/frontend/clusters/components/applications_spec.js index 221ebb143be..1d8984cea0a 100644 --- a/spec/frontend/clusters/components/applications_spec.js +++ b/spec/frontend/clusters/components/applications_spec.js @@ -85,7 +85,44 @@ 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(); + }); + }); + + 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', () => { 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/frontend/ide/components/preview/clientside_spec.js b/spec/frontend/ide/components/preview/clientside_spec.js new file mode 100644 index 00000000000..dfc76628d0c --- /dev/null +++ b/spec/frontend/ide/components/preview/clientside_spec.js @@ -0,0 +1,318 @@ +import Vuex from 'vuex'; +import { GlLoadingIcon } from '@gitlab/ui'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import smooshpack from 'smooshpack'; +import Clientside from '~/ide/components/preview/clientside.vue'; + +jest.mock('smooshpack', () => ({ + Manager: jest.fn(), +})); + +const localVue = createLocalVue(); +localVue.use(Vuex); + +const dummyPackageJson = () => ({ + raw: JSON.stringify({ + main: 'index.js', + }), +}); + +describe('IDE clientside preview', () => { + let wrapper; + let store; + const storeActions = { + getFileData: jest.fn().mockReturnValue(Promise.resolve({})), + getRawFileData: jest.fn().mockReturnValue(Promise.resolve('')), + }; + + const waitForCalls = () => new Promise(setImmediate); + + const createComponent = ({ state, getters } = {}) => { + store = new Vuex.Store({ + state: { + entries: {}, + links: {}, + ...state, + }, + getters: { + packageJson: () => '', + currentProject: () => ({ + visibility: 'public', + }), + ...getters, + }, + actions: storeActions, + }); + + wrapper = shallowMount(Clientside, { + sync: false, + store, + localVue, + }); + }; + + beforeAll(() => { + jest.useFakeTimers(); + }); + + afterAll(() => { + jest.useRealTimers(); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('without main entry', () => { + it('creates sandpack manager', () => { + createComponent(); + expect(smooshpack.Manager).not.toHaveBeenCalled(); + }); + }); + describe('with main entry', () => { + beforeEach(() => { + createComponent({ getters: { packageJson: dummyPackageJson } }); + return wrapper.vm.initPreview(); + }); + + it('creates sandpack manager', () => { + expect(smooshpack.Manager).toHaveBeenCalledWith( + '#ide-preview', + { + files: {}, + entry: '/index.js', + showOpenInCodeSandbox: true, + }, + { + fileResolver: { + isFile: expect.any(Function), + readFile: expect.any(Function), + }, + }, + ); + }); + }); + + describe('computed', () => { + describe('normalizedEntries', () => { + it('returns flattened list of blobs with content', () => { + createComponent({ + state: { + entries: { + 'index.js': { type: 'blob', raw: 'test' }, + 'index2.js': { type: 'blob', content: 'content' }, + tree: { type: 'tree' }, + empty: { type: 'blob' }, + }, + }, + }); + + expect(wrapper.vm.normalizedEntries).toEqual({ + '/index.js': { + code: 'test', + }, + '/index2.js': { + code: 'content', + }, + }); + }); + }); + + describe('mainEntry', () => { + it('returns false when package.json is empty', () => { + createComponent(); + expect(wrapper.vm.mainEntry).toBe(false); + }); + + it('returns main key from package.json', () => { + createComponent({ getters: { packageJson: dummyPackageJson } }); + expect(wrapper.vm.mainEntry).toBe('index.js'); + }); + }); + + describe('showPreview', () => { + it('returns false if no mainEntry', () => { + createComponent(); + expect(wrapper.vm.showPreview).toBe(false); + }); + + it('returns false if loading and mainEntry exists', () => { + createComponent({ getters: { packageJson: dummyPackageJson } }); + wrapper.setData({ loading: true }); + + expect(wrapper.vm.showPreview).toBe(false); + }); + + it('returns true if not loading and mainEntry exists', () => { + createComponent({ getters: { packageJson: dummyPackageJson } }); + wrapper.setData({ loading: false }); + + expect(wrapper.vm.showPreview).toBe(true); + }); + }); + + describe('showEmptyState', () => { + it('returns true if no mainEntry exists', () => { + createComponent(); + wrapper.setData({ loading: false }); + expect(wrapper.vm.showEmptyState).toBe(true); + }); + + it('returns false if loading', () => { + createComponent(); + wrapper.setData({ loading: true }); + + expect(wrapper.vm.showEmptyState).toBe(false); + }); + + it('returns false if not loading and mainEntry exists', () => { + createComponent({ getters: { packageJson: dummyPackageJson } }); + wrapper.setData({ loading: false }); + + expect(wrapper.vm.showEmptyState).toBe(false); + }); + }); + + describe('showOpenInCodeSandbox', () => { + it('returns true when visiblity is public', () => { + createComponent({ getters: { currentProject: () => ({ visibility: 'public' }) } }); + + expect(wrapper.vm.showOpenInCodeSandbox).toBe(true); + }); + + it('returns false when visiblity is private', () => { + createComponent({ getters: { currentProject: () => ({ visibility: 'private' }) } }); + + expect(wrapper.vm.showOpenInCodeSandbox).toBe(false); + }); + }); + + describe('sandboxOpts', () => { + beforeEach(() => { + createComponent({ + state: { + entries: { + 'index.js': { type: 'blob', raw: 'test' }, + 'package.json': dummyPackageJson(), + }, + }, + getters: { + packageJson: dummyPackageJson, + }, + }); + }); + + it('returns sandbox options', () => { + expect(wrapper.vm.sandboxOpts).toEqual({ + files: { + '/index.js': { + code: 'test', + }, + '/package.json': { + code: '{"main":"index.js"}', + }, + }, + entry: '/index.js', + showOpenInCodeSandbox: true, + }); + }); + }); + }); + + describe('methods', () => { + describe('loadFileContent', () => { + beforeEach(() => { + createComponent(); + return wrapper.vm.loadFileContent('package.json'); + }); + + it('calls getFileData', () => { + expect(storeActions.getFileData).toHaveBeenCalledWith( + expect.any(Object), + { + path: 'package.json', + makeFileActive: false, + }, + undefined, // vuex callback + ); + }); + + it('calls getRawFileData', () => { + expect(storeActions.getRawFileData).toHaveBeenCalledWith( + expect.any(Object), + { + path: 'package.json', + }, + undefined, // vuex callback + ); + }); + }); + + describe('update', () => { + beforeEach(() => { + createComponent(); + wrapper.setData({ sandpackReady: true }); + }); + + it('initializes manager if manager is empty', () => { + createComponent({ getters: { packageJson: dummyPackageJson } }); + wrapper.setData({ sandpackReady: true }); + wrapper.vm.update(); + + jest.advanceTimersByTime(250); + + return waitForCalls().then(() => { + expect(smooshpack.Manager).toHaveBeenCalled(); + }); + }); + + it('calls updatePreview', () => { + wrapper.setData({ + manager: { + listener: jest.fn(), + updatePreview: jest.fn(), + }, + }); + wrapper.vm.update(); + + jest.advanceTimersByTime(250); + expect(wrapper.vm.manager.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); + }); + }); + }); + + describe('template', () => { + it('renders ide-preview element when showPreview is true', () => { + createComponent({ getters: { packageJson: dummyPackageJson } }); + wrapper.setData({ loading: false }); + + return wrapper.vm.$nextTick(() => { + expect(wrapper.find('#ide-preview').exists()).toBe(true); + }); + }); + + it('renders empty state', () => { + createComponent(); + wrapper.setData({ loading: false }); + + return wrapper.vm.$nextTick(() => { + expect(wrapper.text()).toContain( + 'Preview your web application using Web IDE client-side evaluation.', + ); + }); + }); + + it('renders loading icon', () => { + createComponent(); + wrapper.setData({ loading: true }); + + return wrapper.vm.$nextTick(() => { + expect(wrapper.find(GlLoadingIcon).exists()).toBe(true); + }); + }); + }); +}); diff --git a/spec/frontend/lib/utils/axios_utils_spec.js b/spec/frontend/lib/utils/axios_utils_spec.js new file mode 100644 index 00000000000..d5c39567f06 --- /dev/null +++ b/spec/frontend/lib/utils/axios_utils_spec.js @@ -0,0 +1,45 @@ +/* eslint-disable promise/catch-or-return */ + +import AxiosMockAdapter from 'axios-mock-adapter'; + +import axios from '~/lib/utils/axios_utils'; + +describe('axios_utils', () => { + let mock; + + beforeEach(() => { + mock = new AxiosMockAdapter(axios); + mock.onAny('/ok').reply(200); + mock.onAny('/err').reply(500); + expect(axios.countActiveRequests()).toBe(0); + }); + + afterEach(() => axios.waitForAll().finally(() => mock.restore())); + + describe('waitForAll', () => { + it('resolves if there are no requests', () => axios.waitForAll()); + + it('waits for all requests to finish', () => { + const handler = jest.fn(); + axios.get('/ok').then(handler); + axios.get('/err').catch(handler); + + return axios.waitForAll().finally(() => { + expect(handler).toHaveBeenCalledTimes(2); + expect(handler.mock.calls[0][0].status).toBe(200); + expect(handler.mock.calls[1][0].response.status).toBe(500); + }); + }); + }); + + describe('waitFor', () => { + it('waits for requests on a specific URL', () => { + const handler = jest.fn(); + axios.get('/ok').finally(handler); + axios.waitFor('/err').finally(() => { + throw new Error('waitFor on /err should not be called'); + }); + return axios.waitFor('/ok'); + }); + }); +}); diff --git a/spec/frontend/mocks/ce/lib/utils/axios_utils.js b/spec/frontend/mocks/ce/lib/utils/axios_utils.js index a3783b91f95..85fad231d28 100644 --- a/spec/frontend/mocks/ce/lib/utils/axios_utils.js +++ b/spec/frontend/mocks/ce/lib/utils/axios_utils.js @@ -1,3 +1,5 @@ +import EventEmitter from 'events'; + const axios = jest.requireActual('~/lib/utils/axios_utils').default; axios.isMock = true; @@ -13,4 +15,64 @@ axios.defaults.adapter = config => { throw error; }; +// Count active requests and provide a way to wait for them +let activeRequests = 0; +const events = new EventEmitter(); +const onRequest = () => { + activeRequests += 1; +}; + +// Use setImmediate to alloow the response interceptor to finish +const onResponse = config => { + activeRequests -= 1; + setImmediate(() => { + events.emit('response', config); + }); +}; + +const subscribeToResponse = (predicate = () => true) => + new Promise(resolve => { + const listener = (config = {}) => { + if (predicate(config)) { + events.off('response', listener); + resolve(config); + } + }; + + events.on('response', listener); + + // If a request has been made synchronously, setImmediate waits for it to be + // processed and the counter incremented. + setImmediate(listener); + }); + +/** + * Registers a callback function to be run after a request to the given URL finishes. + */ +axios.waitFor = url => subscribeToResponse(({ url: configUrl }) => configUrl === url); + +/** + * Registers a callback function to be run after all requests have finished. If there are no requests waiting, the callback is executed immediately. + */ +axios.waitForAll = () => subscribeToResponse(() => activeRequests === 0); + +axios.countActiveRequests = () => activeRequests; + +axios.interceptors.request.use(config => { + onRequest(); + return config; +}); + +// Remove the global counter +axios.interceptors.response.use( + response => { + onResponse(response.config); + return response; + }, + err => { + onResponse(err.config); + return Promise.reject(err); + }, +); + export default axios; diff --git a/spec/frontend/notes/old_notes_spec.js b/spec/frontend/notes/old_notes_spec.js index b57041cf4d1..96133c601aa 100644 --- a/spec/frontend/notes/old_notes_spec.js +++ b/spec/frontend/notes/old_notes_spec.js @@ -49,17 +49,12 @@ describe('Old Notes (~/notes.js)', () => { setTestTimeoutOnce(4000); }); - afterEach(done => { + afterEach(() => { // The Notes component sets a polling interval. Clear it after every run. // Make sure to use jest.runOnlyPendingTimers() instead of runAllTimers(). jest.clearAllTimers(); - setImmediate(() => { - // Wait for any requests to resolve, otherwise we get failures about - // unmocked requests. - mockAxios.restore(); - done(); - }); + return axios.waitForAll().finally(() => mockAxios.restore()); }); it('loads the Notes class into the DOM', () => { diff --git a/spec/helpers/external_link_helper_spec.rb b/spec/helpers/external_link_helper_spec.rb new file mode 100644 index 00000000000..7fc4ef18731 --- /dev/null +++ b/spec/helpers/external_link_helper_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ExternalLinkHelper do + include IconsHelper + + it 'returns external link with icon' do + expect(external_link('https://gitlab.com', 'https://gitlab.com').to_s) + .to eq('<a target="_blank" rel="noopener noreferrer" href="https://gitlab.com">https://gitlab.com <i aria-hidden="true" data-hidden="true" class="fa fa-external-link"></i></a>') + end + + it 'allows options when creating external link with icon' do + expect(external_link('https://gitlab.com', 'https://gitlab.com', { "data-foo": "bar", class: "externalLink" }).to_s) + .to eq('<a target="_blank" rel="noopener noreferrer" data-foo="bar" class="externalLink" href="https://gitlab.com">https://gitlab.com <i aria-hidden="true" data-hidden="true" class="fa fa-external-link"></i></a>') + end +end 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/ide/components/preview/clientside_spec.js b/spec/javascripts/ide/components/preview/clientside_spec.js deleted file mode 100644 index b9bf5c51ffe..00000000000 --- a/spec/javascripts/ide/components/preview/clientside_spec.js +++ /dev/null @@ -1,363 +0,0 @@ -import Vue from 'vue'; -import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; -import { createStore } from '~/ide/stores'; -import Clientside from '~/ide/components/preview/clientside.vue'; -import timeoutPromise from 'spec/helpers/set_timeout_promise_helper'; -import { resetStore, file } from '../../helpers'; - -describe('IDE clientside preview', () => { - let vm; - let Component; - - beforeAll(() => { - Component = Vue.extend(Clientside); - }); - - beforeEach(done => { - const store = createStore(); - - Vue.set(store.state.entries, 'package.json', { - ...file('package.json'), - }); - Vue.set(store.state, 'currentProjectId', 'gitlab-ce'); - Vue.set(store.state.projects, 'gitlab-ce', { - visibility: 'public', - }); - - vm = createComponentWithStore(Component, store); - - spyOn(vm, 'getFileData').and.returnValue(Promise.resolve()); - spyOn(vm, 'getRawFileData').and.returnValue(Promise.resolve('')); - spyOn(vm, 'initManager'); - - vm.$mount(); - - timeoutPromise() - .then(() => vm.$nextTick()) - .then(done) - .catch(done.fail); - }); - - afterEach(() => { - vm.$destroy(); - resetStore(vm.$store); - }); - - describe('without main entry', () => { - it('creates sandpack manager', () => { - expect(vm.initManager).not.toHaveBeenCalled(); - }); - }); - - describe('with main entry', () => { - beforeEach(done => { - Vue.set( - vm.$store.state.entries['package.json'], - 'raw', - JSON.stringify({ - main: 'index.js', - }), - ); - - vm.$nextTick() - .then(() => vm.initPreview()) - .then(vm.$nextTick) - .then(done) - .catch(done.fail); - }); - - it('creates sandpack manager', () => { - expect(vm.initManager).toHaveBeenCalledWith( - '#ide-preview', - { - files: jasmine.any(Object), - entry: '/index.js', - showOpenInCodeSandbox: true, - }, - { - fileResolver: { - isFile: jasmine.any(Function), - readFile: jasmine.any(Function), - }, - }, - ); - }); - }); - - describe('computed', () => { - describe('normalizedEntries', () => { - beforeEach(done => { - vm.$store.state.entries['index.js'] = { - ...file('index.js'), - type: 'blob', - raw: 'test', - }; - vm.$store.state.entries['index2.js'] = { - ...file('index2.js'), - type: 'blob', - content: 'content', - }; - vm.$store.state.entries.tree = { - ...file('tree'), - type: 'tree', - }; - vm.$store.state.entries.empty = { - ...file('empty'), - type: 'blob', - }; - - vm.$nextTick(done); - }); - - it('returns flattened list of blobs with content', () => { - expect(vm.normalizedEntries).toEqual({ - '/index.js': { - code: 'test', - }, - '/index2.js': { - code: 'content', - }, - }); - }); - }); - - describe('mainEntry', () => { - it('returns false when package.json is empty', () => { - expect(vm.mainEntry).toBe(false); - }); - - it('returns main key from package.json', done => { - Vue.set( - vm.$store.state.entries['package.json'], - 'raw', - JSON.stringify({ - main: 'index.js', - }), - ); - - vm.$nextTick(() => { - expect(vm.mainEntry).toBe('index.js'); - - done(); - }); - }); - }); - - describe('showPreview', () => { - it('returns false if no mainEntry', () => { - expect(vm.showPreview).toBe(false); - }); - - it('returns false if loading', done => { - Vue.set( - vm.$store.state.entries['package.json'], - 'raw', - JSON.stringify({ - main: 'index.js', - }), - ); - vm.loading = true; - - vm.$nextTick(() => { - expect(vm.showPreview).toBe(false); - - done(); - }); - }); - - it('returns true if not loading and mainEntry exists', done => { - Vue.set( - vm.$store.state.entries['package.json'], - 'raw', - JSON.stringify({ - main: 'index.js', - }), - ); - vm.loading = false; - - vm.$nextTick(() => { - expect(vm.showPreview).toBe(true); - - done(); - }); - }); - }); - - describe('showEmptyState', () => { - it('returns true if no mainEnry exists', () => { - expect(vm.showEmptyState).toBe(true); - }); - - it('returns false if loading', done => { - Vue.set( - vm.$store.state.entries['package.json'], - 'raw', - JSON.stringify({ - main: 'index.js', - }), - ); - vm.loading = true; - - vm.$nextTick(() => { - expect(vm.showEmptyState).toBe(false); - - done(); - }); - }); - - it('returns false if not loading and mainEntry exists', done => { - Vue.set( - vm.$store.state.entries['package.json'], - 'raw', - JSON.stringify({ - main: 'index.js', - }), - ); - vm.loading = false; - - vm.$nextTick(() => { - expect(vm.showEmptyState).toBe(false); - - done(); - }); - }); - }); - - describe('showOpenInCodeSandbox', () => { - it('returns true when visiblity is public', () => { - expect(vm.showOpenInCodeSandbox).toBe(true); - }); - - it('returns false when visiblity is private', done => { - vm.$store.state.projects['gitlab-ce'].visibility = 'private'; - - vm.$nextTick(() => { - expect(vm.showOpenInCodeSandbox).toBe(false); - - done(); - }); - }); - }); - - describe('sandboxOpts', () => { - beforeEach(done => { - vm.$store.state.entries['index.js'] = { - ...file('index.js'), - type: 'blob', - raw: 'test', - }; - Vue.set( - vm.$store.state.entries['package.json'], - 'raw', - JSON.stringify({ - main: 'index.js', - }), - ); - - vm.$nextTick(done); - }); - - it('returns sandbox options', () => { - expect(vm.sandboxOpts).toEqual({ - files: { - '/index.js': { - code: 'test', - }, - '/package.json': { - code: '{"main":"index.js"}', - }, - }, - entry: '/index.js', - showOpenInCodeSandbox: true, - }); - }); - }); - }); - - describe('methods', () => { - describe('loadFileContent', () => { - it('calls getFileData', () => { - expect(vm.getFileData).toHaveBeenCalledWith({ - path: 'package.json', - makeFileActive: false, - }); - }); - - it('calls getRawFileData', () => { - expect(vm.getRawFileData).toHaveBeenCalledWith({ path: 'package.json' }); - }); - }); - - describe('update', () => { - beforeEach(() => { - jasmine.clock().install(); - - vm.sandpackReady = true; - vm.manager.updatePreview = jasmine.createSpy('updatePreview'); - vm.manager.listener = jasmine.createSpy('updatePreview'); - }); - - afterEach(() => { - jasmine.clock().uninstall(); - }); - - it('calls initPreview if manager is empty', () => { - spyOn(vm, 'initPreview'); - vm.manager = {}; - - vm.update(); - - jasmine.clock().tick(250); - - expect(vm.initPreview).toHaveBeenCalled(); - }); - - it('calls updatePreview', () => { - vm.update(); - - jasmine.clock().tick(250); - - expect(vm.manager.updatePreview).toHaveBeenCalledWith(vm.sandboxOpts); - }); - }); - }); - - describe('template', () => { - it('renders ide-preview element when showPreview is true', done => { - Vue.set( - vm.$store.state.entries['package.json'], - 'raw', - JSON.stringify({ - main: 'index.js', - }), - ); - vm.loading = false; - - vm.$nextTick(() => { - expect(vm.$el.querySelector('#ide-preview')).not.toBe(null); - done(); - }); - }); - - it('renders empty state', done => { - vm.loading = false; - - vm.$nextTick(() => { - expect(vm.$el.textContent).toContain( - 'Preview your web application using Web IDE client-side evaluation.', - ); - - done(); - }); - }); - - it('renders loading icon', done => { - vm.loading = true; - - vm.$nextTick(() => { - expect(vm.$el.querySelector('.loading-container')).not.toBe(null); - done(); - }); - }); - }); -}); diff --git a/spec/lib/api/support/git_access_actor_spec.rb b/spec/lib/api/support/git_access_actor_spec.rb new file mode 100644 index 00000000000..63f5966faea --- /dev/null +++ b/spec/lib/api/support/git_access_actor_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::Support::GitAccessActor do + let(:user) { nil } + let(:key) { nil } + + subject { described_class.new(user: user, key: key) } + + describe '.from_params' do + context 'with params that are valid' do + it 'returns an instance of API::Support::GitAccessActor' do + params = { key_id: create(:key).id } + + expect(described_class.from_params(params)).to be_instance_of(described_class) + end + end + + context 'with params that are invalid' do + it 'returns nil' do + expect(described_class.from_params({})).to be_nil + end + end + end + + describe 'attributes' do + describe '#user' do + context 'when initialized with a User' do + let(:user) { create(:user) } + + it 'returns the User' do + expect(subject.user).to eq(user) + end + end + + context 'when initialized with a Key' do + let(:user_for_key) { create(:user) } + let(:key) { create(:key, user: user_for_key) } + + it 'returns the User associated to the Key' do + expect(subject.user).to eq(user_for_key) + end + end + end + end + + describe '#key_or_user' do + context 'when params contains a :key_id' do + it 'is an instance of Key' do + key = create(:key) + params = { key_id: key.id } + + expect(described_class.from_params(params).key_or_user).to eq(key) + end + end + + context 'when params contains a :user_id' do + it 'is an instance of User' do + user = create(:user) + params = { user_id: user.id } + + expect(described_class.from_params(params).key_or_user).to eq(user) + end + end + + context 'when params contains a :username' do + it 'is an instance of User (via UserFinder)' do + user = create(:user) + params = { username: user.username } + + expect(described_class.from_params(params).key_or_user).to eq(user) + end + end + end + + describe '#username' do + context 'when initialized with a User' do + let(:user) { create(:user) } + + it 'returns the username' do + expect(subject.username).to eq(user.username) + end + end + + context 'when initialized with a Key' do + let(:key) { create(:key, user: user_for_key) } + + context 'that has no User associated' do + let(:user_for_key) { nil } + + it 'returns nil' do + expect(subject.username).to be_nil + end + end + + context 'that has a User associated' do + let(:user_for_key) { create(:user) } + + it 'returns the username of the User associated to the Key' do + expect(subject.username).to eq(user_for_key.username) + end + end + end + end + + describe '#update_last_used_at!' do + context 'when initialized with a User' do + let(:user) { create(:user) } + + it 'does nothing' do + expect(user).not_to receive(:update_last_used_at) + + subject.update_last_used_at! + end + end + + context 'when initialized with a Key' do + let(:key) { create(:key) } + + it 'updates update_last_used_at' do + expect(key).to receive(:update_last_used_at) + + subject.update_last_used_at! + end + end + end +end 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/checks/diff_check_spec.rb b/spec/lib/gitlab/checks/diff_check_spec.rb index a341dfa5636..b9134b8d6ab 100644 --- a/spec/lib/gitlab/checks/diff_check_spec.rb +++ b/spec/lib/gitlab/checks/diff_check_spec.rb @@ -20,9 +20,8 @@ describe Gitlab::Checks::DiffCheck do allow(project).to receive(:lfs_enabled?).and_return(false) end - it 'skips the validation' do - expect(subject).not_to receive(:validate_diff) - expect(subject).not_to receive(:validate_file_paths) + it 'does not invoke :lfs_file_locks_validation' do + expect(subject).not_to receive(:lfs_file_locks_validation) subject.validate! end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 91c559dcd9b..cf496b79a62 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -50,6 +50,32 @@ module Gitlab end end + describe 'interruptible entry' do + describe 'interruptible job' do + let(:config) do + YAML.dump(rspec: { script: 'rspec', interruptible: true }) + end + + it { expect(subject[:interruptible]).to be_truthy } + end + + describe 'interruptible job with default value' do + let(:config) do + YAML.dump(rspec: { script: 'rspec' }) + end + + it { expect(subject).not_to have_key(:interruptible) } + end + + describe 'uninterruptible job' do + let(:config) do + YAML.dump(rspec: { script: 'rspec', interruptible: false }) + end + + it { expect(subject[:interruptible]).to be_falsy } + end + end + describe 'retry entry' do context 'when retry count is specified' do let(:config) do diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index 2990594c538..710564b7540 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -168,9 +168,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/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 516e62c4728..d34c6d2421b 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -329,6 +329,7 @@ CommitStatus: - failure_reason - scheduled_at - upstream_pipeline_id +- interruptible Ci::Variable: - id - project_id @@ -467,6 +468,7 @@ ProtectedBranch: - name - created_at - updated_at +- code_owner_approval_required ProtectedTag: - id - project_id diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index e70fde09edd..45e74597a2e 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -204,17 +204,17 @@ describe Gitlab::Metrics::Transaction do end it 'allows tracking of custom tags' do - transaction.add_event(:meow, animal: 'cat') + transaction.add_event(:bau, animal: 'dog') - expect(metric.tags).to eq(event: :meow, animal: 'cat') + expect(metric.tags).to eq(event: :bau, animal: 'dog') end context 'with sensitive tags' do before do - transaction.add_event(:meow, **sensitive_tags.merge(sane: 'yes')) + transaction.add_event(:baubau, **sensitive_tags.merge(sane: 'yes')) end - it_behaves_like 'tag filter', event: :meow, sane: 'yes' + it_behaves_like 'tag filter', event: :baubau, sane: 'yes' end end diff --git a/spec/lib/gitlab/metrics/web_transaction_spec.rb b/spec/lib/gitlab/metrics/web_transaction_spec.rb index 2b35f07cc0d..21a762dbf25 100644 --- a/spec/lib/gitlab/metrics/web_transaction_spec.rb +++ b/spec/lib/gitlab/metrics/web_transaction_spec.rb @@ -253,11 +253,11 @@ describe Gitlab::Metrics::WebTransaction do end it 'allows tracking of custom tags' do - transaction.add_event(:meow, animal: 'cat') + transaction.add_event(:bau, animal: 'dog') metric = transaction.metrics[0] - expect(metric.tags).to eq(event: :meow, animal: 'cat') + expect(metric.tags).to eq(event: :bau, animal: 'dog') end end end 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/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/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index a217dc42537..09fb2fff521 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -261,6 +261,18 @@ describe HasStatus do end end + describe '.alive_or_scheduled' do + subject { CommitStatus.alive_or_scheduled } + + %i[running pending preparing created scheduled].each do |status| + it_behaves_like 'containing the job', status + end + + %i[failed success canceled skipped].each do |status| + it_behaves_like 'not containing the job', status + end + end + describe '.created_or_pending' do subject { CommitStatus.created_or_pending } 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 fad865a4811..6cec93a53fd 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -220,11 +220,11 @@ describe Ci::CreatePipelineService do expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) end - it 'does not cancel running outdated pipelines' do + it 'cancels running outdated pipelines' do pipeline_on_previous_commit.run - execute_service + head_pipeline = execute_service - expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'running', auto_canceled_by_id: nil) + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: head_pipeline.id) end it 'cancel created outdated pipelines' do @@ -243,6 +243,202 @@ describe Ci::CreatePipelineService do expect(pending_pipeline.reload).to have_attributes(status: 'pending', auto_canceled_by_id: nil) end + + context 'when the interruptible attribute is' do + context 'not defined' do + before do + config = YAML.dump(rspec: { script: 'echo' }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_nil + end + end + + context 'set to true' do + before do + config = YAML.dump(rspec: { script: 'echo', interruptible: true }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_truthy + end + end + + context 'set to false' do + before do + config = YAML.dump(rspec: { script: 'echo', interruptible: false }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is not cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_falsy + end + end + + context 'not defined, but an environment is' do + before do + config = YAML.dump(rspec: { script: 'echo', environment: { name: "review/$CI_COMMIT_REF_NAME" } }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is not cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_nil + end + end + + context 'overriding the environment definition' do + before do + config = YAML.dump(rspec: { script: 'echo', environment: { name: "review/$CI_COMMIT_REF_NAME" }, interruptible: true }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_truthy + end + end + end + + context 'interruptible builds' do + before do + stub_ci_pipeline_yaml_file(YAML.dump(config)) + end + + let(:config) do + { + stages: %w[stage1 stage2 stage3 stage4], + + build_1_1: { + stage: 'stage1', + script: 'echo' + }, + build_1_2: { + stage: 'stage1', + script: 'echo', + interruptible: true + }, + build_2_1: { + stage: 'stage2', + script: 'echo', + when: 'delayed', + start_in: '10 minutes' + }, + build_3_1: { + stage: 'stage3', + script: 'echo', + interruptible: false + }, + build_4_1: { + stage: 'stage4', + script: 'echo' + } + } + end + + it 'properly configures interruptible status' do + interruptible_status = + pipeline_on_previous_commit + .builds + .joins(:metadata) + .pluck(:name, 'ci_builds_metadata.interruptible') + + expect(interruptible_status).to contain_exactly( + ['build_1_1', nil], + ['build_1_2', true], + ['build_2_1', nil], + ['build_3_1', false], + ['build_4_1', nil] + ) + end + + context 'when only interruptible builds are running' do + context 'when build marked explicitly by interruptible is running' do + it 'cancels running outdated pipelines' do + pipeline_on_previous_commit + .builds + .find_by_name('build_1_2') + .run! + + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'canceled', auto_canceled_by_id: pipeline.id) + end + end + + context 'when build that is not marked as interruptible is running' do + it 'cancels running outdated pipelines' do + pipeline_on_previous_commit + .builds + .find_by_name('build_2_1') + .tap(&:enqueue!) + .run! + + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'canceled', auto_canceled_by_id: pipeline.id) + end + end + end + + context 'when an uninterruptible build is running' do + it 'does not cancel running outdated pipelines' do + pipeline_on_previous_commit + .builds + .find_by_name('build_3_1') + .tap(&:enqueue!) + .run! + + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'running', auto_canceled_by_id: nil) + end + end + + context 'when an build is waiting on an interruptible scheduled task' do + it 'cancels running outdated pipelines' do + allow(Ci::BuildScheduleWorker).to receive(:perform_at) + + pipeline_on_previous_commit + .builds + .find_by_name('build_2_1') + .schedule! + + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'canceled', auto_canceled_by_id: pipeline.id) + end + end + + context 'when a uninterruptible build has finished' do + it 'does not cancel running outdated pipelines' do + pipeline_on_previous_commit + .builds + .find_by_name('build_3_1') + .success! + + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'running', auto_canceled_by_id: nil) + end + end + end end context 'auto-cancel disabled' do 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/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/tmp/.gitignore b/tmp/.gitignore new file mode 100644 index 00000000000..a383b23fb78 --- /dev/null +++ b/tmp/.gitignore @@ -0,0 +1,5 @@ +* +!*/ +!.gitignore +!.gitkeep +/tests/ |
