diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-01 12:05:59 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-01 12:05:59 +0000 |
commit | 9e27f0d920cc3891fa7644c5cc0bc280c519fb20 (patch) | |
tree | 9784dd99270f2009159b19077412bf83d13123a4 | |
parent | 1bab0ba591263cd739af2d2c7c3f1b03678a59b6 (diff) | |
download | gitlab-ce-9e27f0d920cc3891fa7644c5cc0bc280c519fb20.tar.gz |
Add latest changes from gitlab-org/gitlab@master
58 files changed, 910 insertions, 187 deletions
diff --git a/app/assets/javascripts/boards/components/issue_card_inner.vue b/app/assets/javascripts/boards/components/issue_card_inner.vue index 2acd92069ca..1d53a21c8ac 100644 --- a/app/assets/javascripts/boards/components/issue_card_inner.vue +++ b/app/assets/javascripts/boards/components/issue_card_inner.vue @@ -104,13 +104,6 @@ export default { helpLink() { return boardsStore.scopedLabels.helpLink; }, - validIssueWeight() { - if (_.isNumber(this.issue.weight)) { - return this.issue.weight >= 0; - } - - return false; - }, }, methods: { isIndexLessThanlimit(index) { diff --git a/app/assets/javascripts/boards/mixins/issue_card_inner.js b/app/assets/javascripts/boards/mixins/issue_card_inner.js index 8000237da6d..04e971b756d 100644 --- a/app/assets/javascripts/boards/mixins/issue_card_inner.js +++ b/app/assets/javascripts/boards/mixins/issue_card_inner.js @@ -1,4 +1,9 @@ export default { + computed: { + validIssueWeight() { + return false; + }, + }, methods: { filterByWeight() {}, }, diff --git a/app/assets/javascripts/jobs/components/job_container_item.vue b/app/assets/javascripts/jobs/components/job_container_item.vue index a55dffbe488..7bd299bcfa0 100644 --- a/app/assets/javascripts/jobs/components/job_container_item.vue +++ b/app/assets/javascripts/jobs/components/job_container_item.vue @@ -54,7 +54,7 @@ export default { :href="job.status.details_path" :title="tooltipText" data-boundary="viewport" - class="js-job-link" + class="js-job-link d-flex" > <icon v-if="isActive" @@ -64,7 +64,7 @@ export default { <ci-icon :status="job.status" /> - <span>{{ job.name ? job.name : job.id }}</span> + <span class="text-truncate w-100">{{ job.name ? job.name : job.id }}</span> <icon v-if="job.retried" name="retry" class="js-retry-icon" /> </gl-link> diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index 73166940146..89fd160b575 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -308,12 +308,8 @@ } a { - display: block; padding: $gl-padding 10px $gl-padding 40px; width: 270px; - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; &:hover { color: $gl-text-color; diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 7577112cb0e..b2c1d0b6dc5 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -1001,6 +1001,14 @@ pre.light-well { } } + &:not(.with-pipeline-status) { + .icon-wrapper:first-of-type { + @include media-breakpoint-up(lg) { + margin-left: $gl-padding-32; + } + } + } + .ci-status-link { display: inline-flex; } diff --git a/app/models/ci/group.rb b/app/models/ci/group.rb index 9b2c3c807ac..0e05318b253 100644 --- a/app/models/ci/group.rb +++ b/app/models/ci/group.rb @@ -9,6 +9,7 @@ module Ci # class Group include StaticModel + include Gitlab::Utils::StrongMemoize attr_reader :stage, :name, :jobs @@ -21,7 +22,17 @@ module Ci end def status - @status ||= commit_statuses.status + strong_memoize(:status) do + if Feature.enabled?(:ci_composite_status, default_enabled: false) + Gitlab::Ci::Status::Composite + .new(@jobs) + .status + else + CommitStatus + .where(id: @jobs) + .legacy_status + end + end end def detailed_status(current_user) @@ -40,11 +51,5 @@ module Ci self.new(stage, name: group_name, jobs: grouped_statuses) end end - - private - - def commit_statuses - @commit_statuses ||= CommitStatus.where(id: jobs.map(&:id)) - end end end diff --git a/app/models/ci/legacy_stage.rb b/app/models/ci/legacy_stage.rb index 930c8a71453..2fd369c9aff 100644 --- a/app/models/ci/legacy_stage.rb +++ b/app/models/ci/legacy_stage.rb @@ -14,7 +14,8 @@ module Ci @pipeline = pipeline @name = name @status = status - @warnings = warnings + # support ints and booleans + @has_warnings = ActiveRecord::Type::Boolean.new.cast(warnings) end def groups @@ -30,7 +31,7 @@ module Ci end def status - @status ||= statuses.latest.status + @status ||= statuses.latest.slow_composite_status end def detailed_status(current_user) @@ -52,11 +53,12 @@ module Ci end def has_warnings? - if @warnings.is_a?(Integer) - @warnings > 0 - else - statuses.latest.failed_but_allowed.any? + # lazilly calculate the warnings + if @has_warnings.nil? + @has_warnings = statuses.latest.failed_but_allowed.any? end + + @has_warnings end def manual_playable? diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 9a96429d3a9..7fa290610aa 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -386,13 +386,12 @@ module Ci end end - def legacy_stages + def legacy_stages_using_sql # TODO, this needs refactoring, see gitlab-foss#26481. - stages_query = statuses .group('stage').select(:stage).order('max(stage_idx)') - status_sql = statuses.latest.where('stage=sg.stage').status_sql + status_sql = statuses.latest.where('stage=sg.stage').legacy_status_sql warnings_sql = statuses.latest.select('COUNT(*)') .where('stage=sg.stage').failed_but_allowed.to_sql @@ -405,6 +404,30 @@ module Ci end end + def legacy_stages_using_composite_status + stages = statuses.latest + .order(:stage_idx, :stage) + .group_by(&:stage) + + stages.map do |stage_name, jobs| + composite_status = Gitlab::Ci::Status::Composite + .new(jobs) + + Ci::LegacyStage.new(self, + name: stage_name, + status: composite_status.status, + warnings: composite_status.warnings?) + end + end + + def legacy_stages + if Feature.enabled?(:ci_composite_status, default_enabled: false) + legacy_stages_using_composite_status + else + legacy_stages_using_sql + end + end + def valid_commit_sha if self.sha == Gitlab::Git::BLANK_SHA self.errors.add(:sha, " cant be 00000000 (branch removal)") @@ -635,7 +658,8 @@ module Ci def update_status retry_optimistic_lock(self) do - case latest_builds_status.to_s + new_status = latest_builds_status.to_s + case new_status when 'created' then nil when 'preparing' then prepare when 'pending' then enqueue @@ -648,7 +672,7 @@ module Ci when 'scheduled' then delay else raise HasStatus::UnknownStatusError, - "Unknown status `#{latest_builds_status}`" + "Unknown status `#{new_status}`" end end end @@ -907,7 +931,7 @@ module Ci def latest_builds_status return 'failed' unless yaml_errors.blank? - statuses.latest.status || 'skipped' + statuses.latest.slow_composite_status || 'skipped' end def keep_around_commits diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index d90339d90dc..77ac8bfe875 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -78,7 +78,8 @@ module Ci def update_status retry_optimistic_lock(self) do - case statuses.latest.status + new_status = latest_stage_status.to_s + case new_status when 'created' then nil when 'preparing' then prepare when 'pending' then enqueue @@ -91,7 +92,7 @@ module Ci when 'skipped', nil then skip else raise HasStatus::UnknownStatusError, - "Unknown status `#{statuses.latest.status}`" + "Unknown status `#{new_status}`" end end end @@ -124,5 +125,9 @@ module Ci def manual_playable? blocked? || skipped? end + + def latest_stage_status + statuses.latest.slow_composite_status || 'skipped' + end end end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 5d9d3179f9d..39a6247b3b2 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -48,6 +48,10 @@ class CommitStatus < ApplicationRecord scope :processables, -> { where(type: %w[Ci::Build Ci::Bridge]) } scope :for_ids, -> (ids) { where(id: ids) } + scope :with_preloads, -> do + preload(:project, :user) + end + scope :with_needs, -> (names = nil) do needs = Ci::BuildNeed.scoped_build.select(1) needs = needs.where(name: names) if names @@ -161,11 +165,11 @@ class CommitStatus < ApplicationRecord end def self.status_for_prior_stages(index) - before_stage(index).latest.status || 'success' + before_stage(index).latest.slow_composite_status || 'success' end def self.status_for_names(names) - where(name: names).latest.status || 'success' + where(name: names).latest.slow_composite_status || 'success' end def locking_enabled? diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index bcbbb27a9a8..c01fb4740e5 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -10,6 +10,8 @@ module HasStatus ACTIVE_STATUSES = %w[preparing pending running].freeze COMPLETED_STATUSES = %w[success failed canceled skipped].freeze ORDERED_STATUSES = %w[failed preparing pending running manual scheduled canceled success skipped created].freeze + PASSED_WITH_WARNINGS_STATUSES = %w[failed canceled].to_set.freeze + EXCLUDE_IGNORED_STATUSES = %w[manual failed canceled].to_set.freeze STATUSES_ENUM = { created: 0, pending: 1, running: 2, success: 3, failed: 4, canceled: 5, skipped: 6, manual: 7, scheduled: 8, preparing: 9 }.freeze @@ -17,7 +19,7 @@ module HasStatus UnknownStatusError = Class.new(StandardError) class_methods do - def status_sql + def legacy_status_sql scope_relevant = respond_to?(:exclude_ignored) ? exclude_ignored : all scope_warnings = respond_to?(:failed_but_allowed) ? failed_but_allowed : none @@ -53,8 +55,22 @@ module HasStatus ) end - def status - all.pluck(status_sql).first + def legacy_status + all.pluck(legacy_status_sql).first + end + + # This method should not be used. + # This method performs expensive calculation of status: + # 1. By plucking all related objects, + # 2. Or executes expensive SQL query + def slow_composite_status + if Feature.enabled?(:ci_composite_status, default_enabled: false) + Gitlab::Ci::Status::Composite + .new(all, with_allow_failure: columns_hash.key?('allow_failure')) + .status + else + legacy_status + end end def started_at diff --git a/app/models/namespace.rb b/app/models/namespace.rb index fb90ddc1048..2fe691bd959 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -319,6 +319,12 @@ class Namespace < ApplicationRecord private def all_projects_with_pages + if all_projects.pages_metadata_not_migrated.exists? + Gitlab::BackgroundMigration::MigratePagesMetadata.new.perform_on_relation( + all_projects.pages_metadata_not_migrated + ) + end + all_projects.with_pages_deployed end diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index 6be3053f637..7903a2182dd 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -194,6 +194,16 @@ class PagesDomain < ApplicationRecord private def pages_deployed? + # TODO: remove once `pages_metadatum` is migrated + # https://gitlab.com/gitlab-org/gitlab/issues/33106 + unless project.pages_metadatum + Gitlab::BackgroundMigration::MigratePagesMetadata + .new + .perform_on_relation(Project.where(id: project_id)) + + project.reset + end + project.pages_metadatum&.deployed? end diff --git a/app/models/project.rb b/app/models/project.rb index 2a7d652678d..318d1473a70 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -433,6 +433,11 @@ class Project < ApplicationRecord joins(:pages_metadatum).merge(ProjectPagesMetadatum.deployed) end + scope :pages_metadata_not_migrated, -> do + left_outer_joins(:pages_metadatum) + .where(project_pages_metadata: { project_id: nil }) + end + enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } chronic_duration_attr :build_timeout_human_readable, :build_timeout, diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 3ecd5390d79..278677edcdf 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -62,6 +62,7 @@ class IssueTrackerService < Service end data_values.reject! { |key| data_fields.changed.include?(key) } + data_values.slice!(*data_fields.attributes.keys) data_fields.assign_attributes(data_values) if data_values.present? self.properties = {} @@ -71,6 +72,10 @@ class IssueTrackerService < Service @legacy_properties_data ||= {} end + def supports_data_fields? + true + end + def data_fields issue_tracker_data || self.build_issue_tracker_data end diff --git a/app/models/service.rb b/app/models/service.rb index 43ed0c7dfaa..305cf7b78a2 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -291,6 +291,12 @@ class Service < ApplicationRecord def self.build_from_template(project_id, template) service = template.dup + + if template.supports_data_fields? + data_fields = template.data_fields.dup + data_fields.service = service + end + service.template = false service.project_id = project_id service.active = false if service.active? && !service.valid? @@ -309,6 +315,11 @@ class Service < ApplicationRecord find_by(template: true) end + # override if needed + def supports_data_fields? + false + end + private def cache_project_has_external_issue_tracker diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index 3b145a65d79..039670f58c8 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -2,6 +2,8 @@ module Ci class ProcessPipelineService < BaseService + include Gitlab::Utils::StrongMemoize + attr_reader :pipeline def execute(pipeline, trigger_build_ids = nil) @@ -33,9 +35,9 @@ module Ci return unless HasStatus::COMPLETED_STATUSES.include?(current_status) - created_processables_in_stage_without_needs(index).select do |build| + created_processables_in_stage_without_needs(index).find_each.select do |build| process_build(build, current_status) - end + end.any? end def process_builds_with_needs(trigger_build_ids) @@ -92,6 +94,7 @@ module Ci def created_processables_in_stage_without_needs(index) created_processables_without_needs + .with_preloads .for_stage(index) end diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index 5432607f82f..67dad9b7a75 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -12,7 +12,9 @@ - css_class += " no-description" if project.description.blank? && !show_last_commit_as_description - cache_key = project_list_cache_key(project, pipeline_status: pipeline_status) - updated_tooltip = time_ago_with_tooltip(project.last_activity_date) -- css_controls_class = compact_mode ? "" : "flex-lg-row justify-content-lg-between" +- show_pipeline_status_icon = pipeline_status && can?(current_user, :read_cross_project) && project.pipeline_status.has_status? && can?(current_user, :read_build, project) +- css_controls_class = compact_mode ? [] : ["flex-lg-row", "justify-content-lg-between"] +- css_controls_class << "with-pipeline-status" if show_pipeline_status_icon - avatar_container_class = project.creator && use_creator_avatar ? '' : 'rect-avatar' %li.project-row.d-flex{ class: css_class } @@ -58,9 +60,9 @@ .description.d-none.d-sm-block.append-right-default = markdown_field(project, :description) - .controls.d-flex.flex-sm-column.align-items-center.align-items-sm-end.flex-wrap.flex-shrink-0.text-secondary{ class: css_controls_class } + .controls.d-flex.flex-sm-column.align-items-center.align-items-sm-end.flex-wrap.flex-shrink-0.text-secondary{ class: css_controls_class.join(" ") } .icon-container.d-flex.align-items-center - - if pipeline_status && can?(current_user, :read_cross_project) && project.pipeline_status.has_status? && can?(current_user, :read_build, project) + - if show_pipeline_status_icon - pipeline_path = pipelines_project_commit_path(project.pipeline_status.project, project.pipeline_status.sha, ref: project.pipeline_status.ref) %span.icon-wrapper.pipeline-status = render 'ci/status/icon', status: project.last_pipeline.detailed_status(current_user), tooltip_placement: 'top', path: pipeline_path diff --git a/changelogs/unreleased/22904-fix-overflow.yml b/changelogs/unreleased/22904-fix-overflow.yml new file mode 100644 index 00000000000..06669d5b548 --- /dev/null +++ b/changelogs/unreleased/22904-fix-overflow.yml @@ -0,0 +1,5 @@ +--- +title: Fixes job overflow in stages dropdown +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/28781-migrate-pages-metadata-on-demand.yml b/changelogs/unreleased/28781-migrate-pages-metadata-on-demand.yml new file mode 100644 index 00000000000..ad478c8895d --- /dev/null +++ b/changelogs/unreleased/28781-migrate-pages-metadata-on-demand.yml @@ -0,0 +1,5 @@ +--- +title: Add index on ci_builds for successful Pages deploys +merge_request: 17204 +author: +type: added diff --git a/changelogs/unreleased/29284-video-preview-not-working.yml b/changelogs/unreleased/29284-video-preview-not-working.yml deleted file mode 100644 index 304700dea30..00000000000 --- a/changelogs/unreleased/29284-video-preview-not-working.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix inline rendering of videos for uploads with uppercase file extensions -merge_request: 17581 -author: -type: fixed diff --git a/changelogs/unreleased/32036-add-some-spacing-offset-to-the-star-icon-to-project-row-in-case-the.yml b/changelogs/unreleased/32036-add-some-spacing-offset-to-the-star-icon-to-project-row-in-case-the.yml new file mode 100644 index 00000000000..6ec9504b1a9 --- /dev/null +++ b/changelogs/unreleased/32036-add-some-spacing-offset-to-the-star-icon-to-project-row-in-case-the.yml @@ -0,0 +1,5 @@ +--- +title: 'Project list: Align star icons' +merge_request: 17833 +author: +type: other diff --git a/changelogs/unreleased/sh-fix-project-export-for-pipelines-for-mrs.yml b/changelogs/unreleased/sh-fix-project-export-for-pipelines-for-mrs.yml new file mode 100644 index 00000000000..1236255e3fd --- /dev/null +++ b/changelogs/unreleased/sh-fix-project-export-for-pipelines-for-mrs.yml @@ -0,0 +1,5 @@ +--- +title: Fix project imports for pipelines for merge requests +merge_request: 17799 +author: +type: fixed diff --git a/db/migrate/20190919040324_add_successfull_pages_deploy_partial_index_on_ci_builds.rb b/db/migrate/20190919040324_add_successfull_pages_deploy_partial_index_on_ci_builds.rb new file mode 100644 index 00000000000..d736b21dddf --- /dev/null +++ b/db/migrate/20190919040324_add_successfull_pages_deploy_partial_index_on_ci_builds.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddSuccessfullPagesDeployPartialIndexOnCiBuilds < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + INDEX_NAME = 'index_ci_builds_on_project_id_for_successfull_pages_deploy' + + def up + add_concurrent_index( + :ci_builds, :project_id, + name: INDEX_NAME, + where: "type='GenericCommitStatus' AND stage='deploy' AND name='pages:deploy' AND status = 'success'" + ) + end + + def down + remove_concurrent_index_by_name :ci_builds, INDEX_NAME + end +end diff --git a/db/schema.rb b/db/schema.rb index ef52143da73..f5eb39d2087 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -643,6 +643,7 @@ ActiveRecord::Schema.define(version: 2019_09_27_074328) do t.index ["name"], name: "index_ci_builds_on_name_for_security_products_values", where: "((name)::text = ANY (ARRAY[('container_scanning'::character varying)::text, ('dast'::character varying)::text, ('dependency_scanning'::character varying)::text, ('license_management'::character varying)::text, ('sast'::character varying)::text]))" t.index ["project_id", "id"], name: "index_ci_builds_on_project_id_and_id" t.index ["project_id", "status"], name: "index_ci_builds_project_id_and_status_for_live_jobs_partial2", where: "(((type)::text = 'Ci::Build'::text) AND ((status)::text = ANY (ARRAY[('running'::character varying)::text, ('pending'::character varying)::text, ('created'::character varying)::text])))" + t.index ["project_id"], name: "index_ci_builds_on_project_id_for_successfull_pages_deploy", where: "(((type)::text = 'GenericCommitStatus'::text) AND ((stage)::text = 'deploy'::text) AND ((name)::text = 'pages:deploy'::text) AND ((status)::text = 'success'::text))" t.index ["protected"], name: "index_ci_builds_on_protected" t.index ["queued_at"], name: "index_ci_builds_on_queued_at" t.index ["runner_id"], name: "index_ci_builds_on_runner_id" diff --git a/doc/security/rack_attack.md b/doc/security/rack_attack.md index 09d29bf3446..51b7d7db3e4 100644 --- a/doc/security/rack_attack.md +++ b/doc/security/rack_attack.md @@ -36,6 +36,9 @@ will be enabled: ### Protected paths throttle +NOTE: **Note:** Omnibus GitLab protected paths throttle is deprecated and is scheduled for removal in +GitLab 13.0. Please refer to [Migrate settings from GitLab 12.3 and earlier](../user/admin_area/settings/protected_paths.md#migrate-settings-from-gitlab-123-and-earlier). + GitLab responds with HTTP status code `429` to POST requests at protected paths that exceed 10 requests per minute per IP address. @@ -124,6 +127,9 @@ The following settings can be configured: **Installations from source** +NOTE: **Note:** Rack Attack initializer was temporarily renamed to `rack_attack_new`, to +support backwards compatibility with the one [Omnibus initializer](https://docs.gitlab.com/omnibus/settings/configuration.html#setting-up-paths-to-be-protected-by-rack-attack). It'll be renamed back to `rack_attack.rb` once Omnibus throttle is removed. Please see the [GitLab issue](https://gitlab.com/gitlab-org/gitlab/issues/29952) for more information. + These settings can be found in `config/initializers/rack_attack.rb`. If you are missing `config/initializers/rack_attack.rb`, the following steps need to be taken in order to enable protection for your GitLab instance: diff --git a/doc/user/admin_area/settings/img/protected_paths.png b/doc/user/admin_area/settings/img/protected_paths.png Binary files differnew file mode 100644 index 00000000000..7aa9124b845 --- /dev/null +++ b/doc/user/admin_area/settings/img/protected_paths.png diff --git a/doc/user/admin_area/settings/index.md b/doc/user/admin_area/settings/index.md index 2a12614e325..ee776faa572 100644 --- a/doc/user/admin_area/settings/index.md +++ b/doc/user/admin_area/settings/index.md @@ -20,6 +20,7 @@ include: - [Visibility and access controls](visibility_and_access_controls.md) - [User and IP rate limits](user_and_ip_rate_limits.md) - [Custom templates repository](instance_template_repository.md) **(PREMIUM)** +- [Protected paths](protected_paths.md) **(CORE ONLY)** NOTE: **Note:** You can change the [first day of the week](../../profile/preferences.md) for the entire GitLab instance diff --git a/doc/user/admin_area/settings/protected_paths.md b/doc/user/admin_area/settings/protected_paths.md new file mode 100644 index 00000000000..21c8d79b138 --- /dev/null +++ b/doc/user/admin_area/settings/protected_paths.md @@ -0,0 +1,76 @@ +--- +type: reference +--- + +# Protected paths **(CORE ONLY)** + +GitLab protects the following paths with Rack Attack by default: + +``` +'/users/password', +'/users/sign_in', +'/api/#{API::API.version}/session.json', +'/api/#{API::API.version}/session', +'/users', +'/users/confirmation', +'/unsubscribes/', +'/import/github/personal_access_token' +``` + +GitLab responds with HTTP status code `429` to POST requests at protected paths +that exceed 10 requests per minute per IP address. + +This header is included in responses to blocked requests: + +``` +Retry-After: 60 +``` + +For example, the following are limited to a maximum 10 requests per minute: + +- User sign-in +- User sign-up (if enabled) +- User password reset + +After 10 requests, the client must wait 60 seconds before it can +try again. + +## Configure using GitLab UI + +> Introduced in [GitLab 12.4](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31246). + +Throttling of protected paths is enabled by default and can be disabled or +customized on **Admin > Network > Protected Paths**, along with these options: + +- Maximum number of requests per period per user. +- Rate limit period in seconds. +- Paths to be protected. + +![protected-paths](img/protected_paths.png) + +Requests over the rate limit are logged into `auth.log`. + +## Migrate settings from GitLab 12.3 and earlier + +Omnibus GitLab protected paths throttle is deprecated and is scheduled for removal in +GitLab 13.0. Please see the [GitLab issue](https://gitlab.com/gitlab-org/gitlab/issues/29952) and the [Omnibus GitLab issue](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/4688) for more information. + +NOTE: **Note:** If Omnibus settings are present, applications settings will be automatically ignored to avoid generating multiple requests blocks. + +To migrate from Omnibus GitLab 12.3 and earlier settings: + +1. Disable the Protected Paths throttle from Omnibus, by changing `rack_attack_enabled` value to `false` on [`rack_attack.rb.erb`](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/files/gitlab-cookbooks/gitlab/templates/default/rack_attack.rb.erb#L18): + + ```ruby + rack_attack_enabled = false + ``` + +1. Customize and enable your protected paths settings by following [Configure using GitLab UI](#configure-using-gitlab-ui) section. + +1. Restart GitLab: + + ```bash + sudo gitlab-ctl restart + ``` + +That's it. Protected paths throttle are now managed by GitLab admin settings. diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index 0fd5c90494e..cfb561481d6 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -316,9 +316,7 @@ This header is included in responses to blocked requests: Retry-After: 60 ``` -Source: - -- Search for `rate_limit_requests_per_period`, `rate_limit_period`, and `rack_attack_protected_paths` in [GitLab.com's current Rails app settings](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/files/gitlab-cookbooks/gitlab/attributes/default.rb). +See [Protected Paths](../admin_area/settings/protected_paths.md) for more details. #### Git and container registry failed authentication ban diff --git a/lib/api/helpers/graphql_helpers.rb b/lib/api/helpers/graphql_helpers.rb index bd60470fbd6..3ddef0c16b3 100644 --- a/lib/api/helpers/graphql_helpers.rb +++ b/lib/api/helpers/graphql_helpers.rb @@ -6,7 +6,7 @@ module API # against the graphql API. Helper code for the graphql server implementation # should be in app/graphql/ or lib/gitlab/graphql/ module GraphqlHelpers - def conditionally_graphql!(fallback:, query:, context: {}, transform: nil) + def run_graphql!(query:, context: {}, transform: nil) result = GitlabSchema.execute(query, context: context) if transform diff --git a/lib/api/version.rb b/lib/api/version.rb index eca1b529094..f79bb3428f2 100644 --- a/lib/api/version.rb +++ b/lib/api/version.rb @@ -19,11 +19,10 @@ module API detail 'This feature was introduced in GitLab 8.13.' end get '/version' do - conditionally_graphql!( + run_graphql!( query: METADATA_QUERY, context: { current_user: current_user }, - transform: ->(result) { result.dig('data', 'metadata') }, - fallback: -> { { version: Gitlab::VERSION, revision: Gitlab.revision } } + transform: ->(result) { result.dig('data', 'metadata') } ) end end diff --git a/lib/banzai/filter/video_link_filter.rb b/lib/banzai/filter/video_link_filter.rb index b3d5d2c95d7..a35b0d7a0b5 100644 --- a/lib/banzai/filter/video_link_filter.rb +++ b/lib/banzai/filter/video_link_filter.rb @@ -8,8 +8,8 @@ module Banzai # a "Download" link in the case the video cannot be played. class VideoLinkFilter < HTML::Pipeline::Filter def call - doc.xpath('descendant-or-self::img[not(ancestor::a)]').each do |el| - el.replace(video_node(doc, el)) if has_video_extension?(el) + doc.xpath(query).each do |el| + el.replace(video_node(doc, el)) end doc @@ -17,10 +17,22 @@ module Banzai private - def has_video_extension?(element) - src_attr = context[:asset_proxy_enabled] ? 'data-canonical-src' : 'src' + def query + @query ||= begin + src_query = UploaderHelper::SAFE_VIDEO_EXT.map do |ext| + "'.#{ext}' = substring(@src, string-length(@src) - #{ext.size})" + end - element.attr(src_attr).downcase.end_with?(*UploaderHelper::SAFE_VIDEO_EXT) + if context[:asset_proxy_enabled].present? + src_query.concat( + UploaderHelper::SAFE_VIDEO_EXT.map do |ext| + "'.#{ext}' = substring(@data-canonical-src, string-length(@data-canonical-src) - #{ext.size})" + end + ) + end + + "descendant-or-self::img[not(ancestor::a) and (#{src_query.join(' or ')})]" + end end def video_node(doc, element) diff --git a/lib/gitlab/background_migration/migrate_pages_metadata.rb b/lib/gitlab/background_migration/migrate_pages_metadata.rb new file mode 100644 index 00000000000..68fd0c17d29 --- /dev/null +++ b/lib/gitlab/background_migration/migrate_pages_metadata.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Class that will insert record into project_pages_metadata + # for each existing project + class MigratePagesMetadata + def perform(start_id, stop_id) + perform_on_relation(Project.where(id: start_id..stop_id)) + end + + def perform_on_relation(relation) + successful_pages_deploy = <<~SQL + SELECT TRUE + FROM ci_builds + WHERE ci_builds.type = 'GenericCommitStatus' + AND ci_builds.status = 'success' + AND ci_builds.stage = 'deploy' + AND ci_builds.name = 'pages:deploy' + AND ci_builds.project_id = projects.id + LIMIT 1 + SQL + + select_from = relation + .select("projects.id", "COALESCE((#{successful_pages_deploy}), FALSE)") + .to_sql + + ActiveRecord::Base.connection_pool.with_connection do |connection| + connection.execute <<~SQL + INSERT INTO project_pages_metadata (project_id, deployed) + #{select_from} + ON CONFLICT (project_id) DO NOTHING + SQL + end + end + end + end +end diff --git a/lib/gitlab/ci/status/composite.rb b/lib/gitlab/ci/status/composite.rb new file mode 100644 index 00000000000..3c00b67911f --- /dev/null +++ b/lib/gitlab/ci/status/composite.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Status + class Composite + include Gitlab::Utils::StrongMemoize + + # This class accepts an array of arrays/hashes/or objects + def initialize(all_statuses, with_allow_failure: true) + unless all_statuses.respond_to?(:pluck) + raise ArgumentError, "all_statuses needs to respond to `.pluck`" + end + + @status_set = Set.new + @status_key = 0 + @allow_failure_key = 1 if with_allow_failure + + consume_all_statuses(all_statuses) + end + + # The status calculation is order dependent, + # 1. In some cases we assume that that status is exact + # if the we only have given statues, + # 2. In other cases we assume that status is of that type + # based on what statuses are no longer valid based on the + # data set that we have + def status + return if none? + + strong_memoize(:status) do + if only_of?(:skipped, :ignored) + 'skipped' + elsif only_of?(:success, :skipped, :success_with_warnings, :ignored) + 'success' + elsif only_of?(:created, :success_with_warnings, :ignored) + 'created' + elsif only_of?(:preparing, :success_with_warnings, :ignored) + 'preparing' + elsif only_of?(:canceled, :success, :skipped, :success_with_warnings, :ignored) + 'canceled' + elsif only_of?(:pending, :created, :skipped, :success_with_warnings, :ignored) + 'pending' + elsif any_of?(:running, :pending) + 'running' + elsif any_of?(:manual) + 'manual' + elsif any_of?(:scheduled) + 'scheduled' + elsif any_of?(:preparing) + 'preparing' + elsif any_of?(:created) + 'running' + else + 'failed' + end + end + end + + def warnings? + @status_set.include?(:success_with_warnings) + end + + private + + def none? + @status_set.empty? + end + + def any_of?(*names) + names.any? { |name| @status_set.include?(name) } + end + + def only_of?(*names) + matching = names.count { |name| @status_set.include?(name) } + matching > 0 && + matching == @status_set.size + end + + def consume_all_statuses(all_statuses) + columns = [] + columns[@status_key] = :status + columns[@allow_failure_key] = :allow_failure if @allow_failure_key + + all_statuses + .pluck(*columns) # rubocop: disable CodeReuse/ActiveRecord + .each(&method(:consume_status)) + end + + def consume_status(description) + # convert `"status"` into `["status"]` + description = Array(description) + + status = + if success_with_warnings?(description) + :success_with_warnings + elsif ignored_status?(description) + :ignored + else + description[@status_key].to_sym + end + + @status_set.add(status) + end + + def success_with_warnings?(status) + @allow_failure_key && + status[@allow_failure_key] && + HasStatus::PASSED_WITH_WARNINGS_STATUSES.include?(status[@status_key]) + end + + def ignored_status?(status) + @allow_failure_key && + status[@allow_failure_key] && + HasStatus::EXCLUDE_IGNORED_STATUSES.include?(status[@status_key]) + end + end + end + end +end diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb index 39a243bd433..017e536c3e7 100644 --- a/lib/gitlab/import_export/project_tree_restorer.rb +++ b/lib/gitlab/import_export/project_tree_restorer.rb @@ -52,6 +52,11 @@ module Gitlab project: restored_project) end + # A Hash of the imported merge request ID -> imported ID. + def merge_requests_mapping + @merge_requests_mapping ||= {} + end + # Loops through the tree of models defined in import_export.yml and # finds them in the imported JSON so they can be instantiated and saved # in the DB. The structure and relationships between models are guessed from @@ -80,10 +85,26 @@ module Gitlab @saved = false unless restored_project.append_or_update_attribute(relation_key, relation_hash) + save_id_mappings(relation_key, relation_hash_batch, relation_hash) + # Restore the project again, extra query that skips holding the AR objects in memory @restored_project = Project.find(@project_id) end + # Older, serialized CI pipeline exports may only have a + # merge_request_id and not the full hash of the merge request. To + # import these pipelines, we need to preserve the mapping between + # the old and new the merge request ID. + def save_id_mappings(relation_key, relation_hash_batch, relation_hash) + return unless relation_key == 'merge_requests' + + relation_hash = Array(relation_hash) + + Array(relation_hash_batch).each_with_index do |raw_data, index| + merge_requests_mapping[raw_data['id']] = relation_hash[index]['id'] + end + end + # Remove project models that became group models as we found them at group level. # This no longer required saving them at the root project level. # For example, in the case of an existing group label that matched the title. @@ -222,6 +243,7 @@ module Gitlab relation_sym: relation_key.to_sym, relation_hash: relation_hash, members_mapper: members_mapper, + merge_requests_mapping: merge_requests_mapping, user: @user, project: @restored_project, excluded_keys: excluded_keys_for_relation(relation_key)) diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 37f625288a9..9ec244b0960 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -55,10 +55,11 @@ module Gitlab relation_name.to_s.constantize end - def initialize(relation_sym:, relation_hash:, members_mapper:, user:, project:, excluded_keys: []) + def initialize(relation_sym:, relation_hash:, members_mapper:, merge_requests_mapping:, user:, project:, excluded_keys: []) @relation_name = self.class.overrides[relation_sym]&.to_sym || relation_sym @relation_hash = relation_hash.except('noteable_id') @members_mapper = members_mapper + @merge_requests_mapping = merge_requests_mapping @user = user @project = project @imported_object_retries = 0 @@ -109,7 +110,10 @@ module Gitlab update_group_references remove_duplicate_assignees - setup_pipeline if @relation_name == :'Ci::Pipeline' + if @relation_name == :'Ci::Pipeline' + update_merge_request_references + setup_pipeline + end reset_tokens! remove_encrypted_attributes! @@ -194,6 +198,28 @@ module Gitlab @relation_hash['group_id'] = @project.namespace_id end + # This code is a workaround for broken project exports that don't + # export merge requests with CI pipelines (i.e. exports that were + # generated from + # https://gitlab.com/gitlab-org/gitlab/merge_requests/17844). + # This method can be removed in GitLab 12.6. + def update_merge_request_references + # If a merge request was properly created, we don't need to fix + # up this export. + return if @relation_hash['merge_request'] + + merge_request_id = @relation_hash['merge_request_id'] + + return unless merge_request_id + + new_merge_request_id = @merge_requests_mapping[merge_request_id] + + return unless new_merge_request_id + + @relation_hash['merge_request_id'] = new_merge_request_id + parsed_relation_hash['merge_request_id'] = new_merge_request_id + end + def reset_tokens! return unless Gitlab::ImportExport.reset_tokens? && TOKEN_RESET_MODELS.include?(@relation_name) diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh index a959ae0549d..9c9d33235c9 100644 --- a/scripts/rspec_helpers.sh +++ b/scripts/rspec_helpers.sh @@ -15,14 +15,14 @@ function retrieve_tests_metadata() { function update_tests_metadata() { echo "{}" > "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" - scripts/merge-reports "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" "knapsack/rspec*_pg9_*.json" + scripts/merge-reports "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" knapsack/rspec*_pg9_*.json if [[ -n "${TESTS_METADATA_S3_BUCKET}" ]]; then scripts/sync-reports put "${TESTS_METADATA_S3_BUCKET}" "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" fi - rm -f "knapsack/rspec*.json" + rm -f knapsack/rspec*.json - scripts/merge-reports "${FLAKY_RSPEC_SUITE_REPORT_PATH}" "rspec_flaky/all_*.json" + scripts/merge-reports "${FLAKY_RSPEC_SUITE_REPORT_PATH}" rspec_flaky/all_*.json export FLAKY_RSPEC_GENERATE_REPORT="1" scripts/prune-old-flaky-specs "${FLAKY_RSPEC_SUITE_REPORT_PATH}" @@ -31,7 +31,7 @@ function update_tests_metadata() { scripts/sync-reports put "${TESTS_METADATA_S3_BUCKET}" "${FLAKY_RSPEC_SUITE_REPORT_PATH}" fi - rm -f "rspec_flaky/all_*.json" "rspec_flaky/new_*.json" + rm -f rspec_flaky/all_*.json rspec_flaky/new_*.json scripts/insert-rspec-profiling-data } diff --git a/spec/features/projects/jobs/user_browses_job_spec.rb b/spec/features/projects/jobs/user_browses_job_spec.rb index 4d8a4812123..82ad08d0ff2 100644 --- a/spec/features/projects/jobs/user_browses_job_spec.rb +++ b/spec/features/projects/jobs/user_browses_job_spec.rb @@ -104,20 +104,20 @@ describe 'User browses a job', :js do it 'displays the failure reason' do wait_for_all_requests within('.builds-container') do - build_link = first('.build-job > a') - expect(build_link['data-original-title']).to eq('test - failed - (unknown failure)') + expect(page).to have_selector( + ".build-job > a[data-original-title='test - failed - (unknown failure)']") end end end context 'when a failed job has been retried' do - let!(:build) { create(:ci_build, :failed, :retried, :trace_artifact, pipeline: pipeline) } + let!(:build_retried) { create(:ci_build, :failed, :retried, :trace_artifact, pipeline: pipeline) } it 'displays the failure reason and retried label' do wait_for_all_requests within('.builds-container') do - build_link = first('.build-job > a') - expect(build_link['data-original-title']).to eq('test - failed - (unknown failure) (retried)') + expect(page).to have_selector( + ".build-job > a[data-original-title='test - failed - (unknown failure) (retried)']") end end end diff --git a/spec/fixtures/lib/gitlab/import_export/project.json b/spec/fixtures/lib/gitlab/import_export/project.json index 4544c38f39a..7d9c8cdef8f 100644 --- a/spec/fixtures/lib/gitlab/import_export/project.json +++ b/spec/fixtures/lib/gitlab/import_export/project.json @@ -6175,6 +6175,8 @@ "finished_at": null, "user_id": 9999, "duration": null, + "source": "push", + "merge_request_id": null, "notes": [ { "id": 999, diff --git a/spec/javascripts/boards/issue_card_spec.js b/spec/javascripts/boards/issue_card_spec.js index 9b5e8afa4ef..9e99f961797 100644 --- a/spec/javascripts/boards/issue_card_spec.js +++ b/spec/javascripts/boards/issue_card_spec.js @@ -286,19 +286,4 @@ describe('Issue card component', () => { .catch(done.fail); }); }); - - describe('weights', () => { - it('shows weight component is greater than 0', () => { - expect(component.$el.querySelector('.board-card-weight')).not.toBeNull(); - }); - - it('shows weight component when weight is 0', done => { - component.issue.weight = 0; - - Vue.nextTick(() => { - expect(component.$el.querySelector('.board-card-weight')).not.toBeNull(); - done(); - }); - }); - }); }); diff --git a/spec/lib/api/helpers/graphql_helpers_spec.rb b/spec/lib/api/helpers/graphql_helpers_spec.rb new file mode 100644 index 00000000000..c775ba6d5e8 --- /dev/null +++ b/spec/lib/api/helpers/graphql_helpers_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::Helpers::GraphqlHelpers do + describe 'run_graphql!' do + let(:query) { '{ metadata { version } }' } + + let(:graphql_helper) do + Class.new do + include API::Helpers::GraphqlHelpers + end.new + end + + context 'when transform function is provided' do + let(:result) { { 'data' => { 'metadata' => { 'version' => '1.0.0' } } } } + + before do + allow(GitlabSchema).to receive(:execute).and_return(result) + end + + it 'returns the expected result' do + expect( + graphql_helper.run_graphql!( + query: query, + transform: ->(result) { result.dig('data', 'metadata') } + ) + ).to eq({ 'version' => '1.0.0' }) + end + end + + context 'when a transform function is not provided' do + let(:result) { double('result') } + + before do + allow(GitlabSchema).to receive(:execute).and_return(result) + end + + it 'returns the expected result' do + expect(graphql_helper.run_graphql!(query: query)).to eq(result) + end + end + end +end diff --git a/spec/lib/banzai/filter/video_link_filter_spec.rb b/spec/lib/banzai/filter/video_link_filter_spec.rb index afcc846ba05..b5be204d680 100644 --- a/spec/lib/banzai/filter/video_link_filter_spec.rb +++ b/spec/lib/banzai/filter/video_link_filter_spec.rb @@ -17,32 +17,27 @@ describe Banzai::Filter::VideoLinkFilter do let(:project) { create(:project, :repository) } - shared_examples 'replaces the image tag with a video tag' do |ext| - it "replaces the image tag 'path/video.#{ext}' with a video tag" do - container = filter(link_to_image("/path/video.#{ext}")).children.first - - expect(container.name).to eq 'div' - expect(container['class']).to eq 'video-container' + context 'when the element src has a video extension' do + UploaderHelper::SAFE_VIDEO_EXT.each do |ext| + it "replaces the image tag 'path/video.#{ext}' with a video tag" do + container = filter(link_to_image("/path/video.#{ext}")).children.first - video, paragraph = container.children + expect(container.name).to eq 'div' + expect(container['class']).to eq 'video-container' - expect(video.name).to eq 'video' - expect(video['src']).to eq "/path/video.#{ext}" + video, paragraph = container.children - expect(paragraph.name).to eq 'p' + expect(video.name).to eq 'video' + expect(video['src']).to eq "/path/video.#{ext}" - link = paragraph.children.first + expect(paragraph.name).to eq 'p' - expect(link.name).to eq 'a' - expect(link['href']).to eq "/path/video.#{ext}" - expect(link['target']).to eq '_blank' - end - end + link = paragraph.children.first - context 'when the element src has a video extension' do - UploaderHelper::SAFE_VIDEO_EXT.each do |ext| - it_behaves_like 'replaces the image tag with a video tag', ext - it_behaves_like 'replaces the image tag with a video tag', ext.upcase + expect(link.name).to eq 'a' + expect(link['href']).to eq "/path/video.#{ext}" + expect(link['target']).to eq '_blank' + end end end diff --git a/spec/lib/gitlab/background_migration/migrate_pages_metadata_spec.rb b/spec/lib/gitlab/background_migration/migrate_pages_metadata_spec.rb new file mode 100644 index 00000000000..d94a312f605 --- /dev/null +++ b/spec/lib/gitlab/background_migration/migrate_pages_metadata_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::MigratePagesMetadata, :migration, schema: 20190919040324 do + let(:projects) { table(:projects) } + + subject(:migrate_pages_metadata) { described_class.new } + + describe '#perform_on_relation' do + let(:namespaces) { table(:namespaces) } + let(:builds) { table(:ci_builds) } + let(:pages_metadata) { table(:project_pages_metadata) } + + it 'marks specified projects with successful pages deployment' do + namespace = namespaces.create!(name: 'gitlab', path: 'gitlab-org') + not_migrated_with_pages = projects.create!(namespace_id: namespace.id, name: 'Not Migrated With Pages') + builds.create!(project_id: not_migrated_with_pages.id, type: 'GenericCommitStatus', status: 'success', stage: 'deploy', name: 'pages:deploy') + + migrated = projects.create!(namespace_id: namespace.id, name: 'Migrated') + pages_metadata.create!(project_id: migrated.id, deployed: true) + + not_migrated_no_pages = projects.create!(namespace_id: namespace.id, name: 'Not Migrated No Pages') + project_not_in_relation_scope = projects.create!(namespace_id: namespace.id, name: 'Other') + + projects_relation = projects.where(id: [not_migrated_with_pages, not_migrated_no_pages, migrated]) + + migrate_pages_metadata.perform_on_relation(projects_relation) + + expect(pages_metadata.find_by_project_id(not_migrated_with_pages.id).deployed).to eq(true) + expect(pages_metadata.find_by_project_id(not_migrated_no_pages.id).deployed).to eq(false) + expect(pages_metadata.find_by_project_id(migrated.id).deployed).to eq(true) + expect(pages_metadata.find_by_project_id(project_not_in_relation_scope.id)).to be_nil + end + end + + describe '#perform' do + it 'creates relation and delegates to #perform_on_relation' do + expect(migrate_pages_metadata).to receive(:perform_on_relation).with(projects.where(id: 3..5)) + + migrate_pages_metadata.perform(3, 5) + end + end +end diff --git a/spec/lib/gitlab/ci/status/composite_spec.rb b/spec/lib/gitlab/ci/status/composite_spec.rb new file mode 100644 index 00000000000..1725d954b92 --- /dev/null +++ b/spec/lib/gitlab/ci/status/composite_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Composite do + set(:pipeline) { create(:ci_pipeline) } + + before(:all) do + @statuses = HasStatus::STATUSES_ENUM.map do |status, idx| + [status, create(:ci_build, pipeline: pipeline, status: status, importing: true)] + end.to_h + + @statuses_with_allow_failure = HasStatus::STATUSES_ENUM.map do |status, idx| + [status, create(:ci_build, pipeline: pipeline, status: status, allow_failure: true, importing: true)] + end.to_h + end + + describe '#status' do + shared_examples 'compares composite with SQL status' do + it 'returns exactly the same result' do + builds = Ci::Build.where(id: all_statuses) + + expect(composite_status.status).to eq(builds.legacy_status) + expect(composite_status.warnings?).to eq(builds.failed_but_allowed.any?) + end + end + + shared_examples 'validate all combinations' do |perms| + HasStatus::STATUSES_ENUM.keys.combination(perms).each do |statuses| + context "with #{statuses.join(",")}" do + it_behaves_like 'compares composite with SQL status' do + let(:all_statuses) do + statuses.map { |status| @statuses[status] } + end + + let(:composite_status) do + described_class.new(all_statuses) + end + end + + HasStatus::STATUSES_ENUM.each do |allow_failure_status, _| + context "and allow_failure #{allow_failure_status}" do + it_behaves_like 'compares composite with SQL status' do + let(:all_statuses) do + statuses.map { |status| @statuses[status] } + + [@statuses_with_allow_failure[allow_failure_status]] + end + + let(:composite_status) do + described_class.new(all_statuses) + end + end + end + end + end + end + end + + it_behaves_like 'validate all combinations', 0 + it_behaves_like 'validate all combinations', 1 + it_behaves_like 'validate all combinations', 2 + end +end diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index fcc79279b6f..c619a2ab237 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -96,6 +96,17 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(Ci::Pipeline.where(ref: nil)).not_to be_empty end + it 'restores pipeline for merge request' do + pipeline = Ci::Pipeline.find_by_sha('048721d90c449b244b7b4c53a9186b04330174ec') + + expect(pipeline).to be_valid + expect(pipeline.tag).to be_falsey + expect(pipeline.source).to eq('merge_request_event') + expect(pipeline.merge_request.id).to be > 0 + expect(pipeline.merge_request.target_branch).to eq('feature') + expect(pipeline.merge_request.source_branch).to eq('feature_conflict') + end + it 'preserves updated_at on issues' do issue = Issue.where(description: 'Aliquam enim illo et possimus.').first diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb index a31f77484d8..51b2fd06b46 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb @@ -3,12 +3,14 @@ require 'spec_helper' describe Gitlab::ImportExport::RelationFactory do let(:project) { create(:project) } let(:members_mapper) { double('members_mapper').as_null_object } + let(:merge_requests_mapping) { {} } let(:user) { create(:admin) } let(:excluded_keys) { [] } let(:created_object) do described_class.create(relation_sym: relation_sym, relation_hash: relation_hash, members_mapper: members_mapper, + merge_requests_mapping: merge_requests_mapping, user: user, project: project, excluded_keys: excluded_keys) diff --git a/spec/models/ci/group_spec.rb b/spec/models/ci/group_spec.rb index 36c65d92840..b3b158a111e 100644 --- a/spec/models/ci/group_spec.rb +++ b/spec/models/ci/group_spec.rb @@ -22,6 +22,32 @@ describe Ci::Group do end end + describe '#status' do + let(:jobs) do + [create(:ci_build, :failed)] + end + + context 'when ci_composite_status is enabled' do + before do + stub_feature_flags(ci_composite_status: true) + end + + it 'returns a failed status' do + expect(subject.status).to eq('failed') + end + end + + context 'when ci_composite_status is disabled' do + before do + stub_feature_flags(ci_composite_status: false) + end + + it 'returns a failed status' do + expect(subject.status).to eq('failed') + end + end + end + describe '#detailed_status' do context 'when there is only one item in the group' do it 'calls the status from the object itself' do diff --git a/spec/models/ci/legacy_stage_spec.rb b/spec/models/ci/legacy_stage_spec.rb index bb812cc0533..477f4036218 100644 --- a/spec/models/ci/legacy_stage_spec.rb +++ b/spec/models/ci/legacy_stage_spec.rb @@ -216,7 +216,7 @@ describe Ci::LegacyStage do context 'when stage has warnings' do context 'when using memoized warnings flag' do context 'when there are warnings' do - let(:stage) { build(:ci_stage, warnings: 2) } + let(:stage) { build(:ci_stage, warnings: true) } it 'returns true using memoized value' do expect(stage).not_to receive(:statuses) @@ -225,22 +225,13 @@ describe Ci::LegacyStage do end context 'when there are no warnings' do - let(:stage) { build(:ci_stage, warnings: 0) } + let(:stage) { build(:ci_stage, warnings: false) } it 'returns false using memoized value' do expect(stage).not_to receive(:statuses) expect(stage).not_to have_warnings end end - - context 'when number of warnings is not a valid value' do - let(:stage) { build(:ci_stage, warnings: true) } - - it 'calculates statuses using database queries' do - expect(stage).to receive(:statuses).and_call_original - expect(stage).not_to have_warnings - end - end end context 'when calculating warnings from statuses' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 3c625784132..0e11c595388 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1136,59 +1136,71 @@ describe Ci::Pipeline, :mailer do end describe '#legacy_stages' do + using RSpec::Parameterized::TableSyntax + subject { pipeline.legacy_stages } - context 'stages list' do - it 'returns ordered list of stages' do - expect(subject.map(&:name)).to eq(%w[build test deploy]) - end + where(:ci_composite_status) do + [false, true] end - context 'stages with statuses' do - let(:statuses) do - subject.map { |stage| [stage.name, stage.status] } + with_them do + before do + stub_feature_flags(ci_composite_status: ci_composite_status) end - it 'returns list of stages with correct statuses' do - expect(statuses).to eq([%w(build failed), - %w(test success), - %w(deploy running)]) + context 'stages list' do + it 'returns ordered list of stages' do + expect(subject.map(&:name)).to eq(%w[build test deploy]) + end end - context 'when commit status is retried' do - before do - create(:commit_status, pipeline: pipeline, - stage: 'build', - name: 'mac', - stage_idx: 0, - status: 'success') - - pipeline.process! + context 'stages with statuses' do + let(:statuses) do + subject.map { |stage| [stage.name, stage.status] } end - it 'ignores the previous state' do - expect(statuses).to eq([%w(build success), + it 'returns list of stages with correct statuses' do + expect(statuses).to eq([%w(build failed), %w(test success), %w(deploy running)]) end - end - end - context 'when there is a stage with warnings' do - before do - create(:commit_status, pipeline: pipeline, - stage: 'deploy', - name: 'prod:2', - stage_idx: 2, - status: 'failed', - allow_failure: true) + context 'when commit status is retried' do + before do + create(:commit_status, pipeline: pipeline, + stage: 'build', + name: 'mac', + stage_idx: 0, + status: 'success') + + pipeline.process! + end + + it 'ignores the previous state' do + expect(statuses).to eq([%w(build success), + %w(test success), + %w(deploy running)]) + end + end end - it 'populates stage with correct number of warnings' do - deploy_stage = pipeline.legacy_stages.third + context 'when there is a stage with warnings' do + before do + create(:commit_status, pipeline: pipeline, + stage: 'deploy', + name: 'prod:2', + stage_idx: 2, + status: 'failed', + allow_failure: true) + end - expect(deploy_stage).not_to receive(:statuses) - expect(deploy_stage).to have_warnings + it 'populates stage with correct number of warnings' do + deploy_stage = pipeline.legacy_stages.third + + expect(deploy_stage).not_to receive(:statuses) + expect(deploy_stage).to have_warnings + end end end end @@ -2326,36 +2338,38 @@ describe Ci::Pipeline, :mailer do describe '#update_status' do context 'when pipeline is empty' do it 'updates does not change pipeline status' do - expect(pipeline.statuses.latest.status).to be_nil + expect(pipeline.statuses.latest.slow_composite_status).to be_nil expect { pipeline.update_status } - .to change { pipeline.reload.status }.to 'skipped' + .to change { pipeline.reload.status } + .from('created') + .to('skipped') end end context 'when updating status to pending' do before do - allow(pipeline) - .to receive_message_chain(:statuses, :latest, :status) - .and_return(:running) + create(:ci_build, pipeline: pipeline, status: :running) end it 'updates pipeline status to running' do expect { pipeline.update_status } - .to change { pipeline.reload.status }.to 'running' + .to change { pipeline.reload.status } + .from('created') + .to('running') end end context 'when updating status to scheduled' do before do - allow(pipeline) - .to receive_message_chain(:statuses, :latest, :status) - .and_return(:scheduled) + create(:ci_build, pipeline: pipeline, status: :scheduled) end it 'updates pipeline status to scheduled' do expect { pipeline.update_status } - .to change { pipeline.reload.status }.to 'scheduled' + .to change { pipeline.reload.status } + .from('created') + .to('scheduled') end end diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index 85cd32fb03a..8827509edda 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -130,7 +130,7 @@ describe Ci::Stage, :models do context 'when statuses status was not recognized' do before do allow(stage) - .to receive_message_chain(:statuses, :latest, :status) + .to receive(:latest_stage_status) .and_return(:unknown) end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 017cca0541e..95e9b0d0f92 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -321,7 +321,7 @@ describe CommitStatus do end it 'returns a correct compound status' do - expect(described_class.all.status).to eq 'running' + expect(described_class.all.slow_composite_status).to eq 'running' end end @@ -331,7 +331,7 @@ describe CommitStatus do end it 'returns status that indicates success' do - expect(described_class.all.status).to eq 'success' + expect(described_class.all.slow_composite_status).to eq 'success' end end @@ -342,7 +342,7 @@ describe CommitStatus do end it 'returns status according to the scope' do - expect(described_class.latest.status).to eq 'success' + expect(described_class.latest.slow_composite_status).to eq 'success' end end end diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index 09fb2fff521..21e4dda6dab 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -3,12 +3,15 @@ require 'spec_helper' describe HasStatus do - describe '.status' do - subject { CommitStatus.status } + describe '.slow_composite_status' do + using RSpec::Parameterized::TableSyntax + + subject { CommitStatus.slow_composite_status } shared_examples 'build status summary' do context 'all successful' do let!(:statuses) { Array.new(2) { create(type, status: :success) } } + it { is_expected.to eq 'success' } end @@ -165,16 +168,26 @@ describe HasStatus do end end - context 'ci build statuses' do - let(:type) { :ci_build } - - it_behaves_like 'build status summary' + where(:ci_composite_status) do + [false, true] end - context 'generic commit statuses' do - let(:type) { :generic_commit_status } + with_them do + before do + stub_feature_flags(ci_composite_status: ci_composite_status) + end + + context 'ci build statuses' do + let(:type) { :ci_build } - it_behaves_like 'build status summary' + it_behaves_like 'build status summary' + end + + context 'generic commit statuses' do + let(:type) { :generic_commit_status } + + it_behaves_like 'build status summary' + end end end @@ -372,8 +385,8 @@ describe HasStatus do end end - describe '.status_sql' do - subject { Ci::Build.status_sql } + describe '.legacy_status_sql' do + subject { Ci::Build.legacy_status_sql } it 'returns SQL' do puts subject diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index e72e272f4d2..a4d60467071 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -928,12 +928,34 @@ describe Namespace do let(:project) { create(:project, namespace: namespace) } context 'when there are pages deployed for the project' do - before do - project.mark_pages_as_deployed + context 'but pages metadata is not migrated' do + before do + generic_commit_status = create(:generic_commit_status, :success, stage: 'deploy', name: 'pages:deploy') + generic_commit_status.update!(project: project) + project.pages_metadatum.destroy! + end + + it 'migrates pages metadata and returns the virual domain' do + virtual_domain = namespace.pages_virtual_domain + + expect(project.reload.pages_metadatum.deployed).to eq(true) + + expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + expect(virtual_domain.lookup_paths).not_to be_empty + end end - it 'returns the virual domain' do - expect(namespace.pages_virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + context 'and pages metadata is migrated' do + before do + project.mark_pages_as_deployed + end + + it 'returns the virual domain' do + virtual_domain = namespace.pages_virtual_domain + + expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + expect(virtual_domain.lookup_paths).not_to be_empty + end end end end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 9ac80f8b795..2e7b2b88432 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -569,7 +569,9 @@ describe PagesDomain do context 'when there are pages deployed for the project' do before do - project.mark_pages_as_deployed + generic_commit_status = create(:generic_commit_status, :success, stage: 'deploy', name: 'pages:deploy') + generic_commit_status.update!(project: project) + project.pages_metadatum.destroy! project.reload end @@ -578,6 +580,12 @@ describe PagesDomain do expect(pages_domain.pages_virtual_domain).to be_an_instance_of(Pages::VirtualDomain) end + + it 'migrates project pages metadata' do + expect { pages_domain.pages_virtual_domain }.to change { + project.reload.pages_metadatum&.deployed + }.from(nil).to(true) + end end end end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 39c1176f238..c3b2e52848c 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -282,7 +282,7 @@ describe JiraService do context 'when data are stored in properties' do let(:properties) { data_params.merge(title: title, description: description) } let!(:service) do - create(:jira_service, :without_properties_callback, properties: properties) + create(:jira_service, :without_properties_callback, properties: properties.merge(additional: 'something')) end it_behaves_like 'issue tracker fields' diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e97e8c58bbd..daccd143b6d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -5107,6 +5107,16 @@ describe Project do end end + describe '.pages_metadata_not_migrated' do + it 'returns only projects that have pages deployed' do + _project_with_pages_metadata_migrated = create(:project) + project_with_pages_metadata_not_migrated = create(:project) + project_with_pages_metadata_not_migrated.pages_metadatum.destroy! + + expect(described_class.pages_metadata_not_migrated).to contain_exactly(project_with_pages_metadata_not_migrated) + end + end + describe '#pages_group_root?' do it 'returns returns true if pages_url is same as pages_group_url' do project = build(:project) diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index d96e1398677..4049ddcff7f 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -78,10 +78,11 @@ describe Service do end describe "Template" do + let(:project) { create(:project) } + describe '.build_from_template' do context 'when template is invalid' do it 'sets service template to inactive when template is invalid' do - project = create(:project) template = build(:prometheus_service, template: true, active: true, properties: {}) template.save(validate: false) @@ -91,6 +92,64 @@ describe Service do expect(service.active).to be false end end + + describe 'build issue tracker from a template' do + let(:title) { 'custom title' } + let(:description) { 'custom description' } + let(:url) { 'http://jira.example.com' } + let(:api_url) { 'http://api-jira.example.com' } + let(:username) { 'jira-username' } + let(:password) { 'jira-password' } + let(:data_params) do + { + url: url, api_url: api_url, + username: username, password: password + } + end + + shared_examples 'service creation from a template' do + it 'creates a correct service' do + service = described_class.build_from_template(project.id, template) + + expect(service).to be_active + expect(service.title).to eq(title) + expect(service.description).to eq(description) + expect(service.url).to eq(url) + expect(service.api_url).to eq(api_url) + expect(service.username).to eq(username) + expect(service.password).to eq(password) + end + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-foss/issues/63084 + context 'when data are stored in properties' do + let(:properties) { data_params.merge(title: title, description: description) } + let!(:template) do + create(:jira_service, :without_properties_callback, template: true, properties: properties.merge(additional: 'something')) + end + + it_behaves_like 'service creation from a template' + end + + context 'when data are stored in separated fields' do + let(:template) do + create(:jira_service, data_params.merge(properties: {}, title: title, description: description, template: true)) + end + + it_behaves_like 'service creation from a template' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { data_params.merge(title: title, description: description) } + let(:template) do + create(:jira_service, :without_properties_callback, active: true, template: true, properties: properties).tap do |service| + create(:jira_tracker_data, data_params.merge(service: service)) + end + end + + it_behaves_like 'service creation from a template' + end + end end describe "for pushover service" do @@ -104,7 +163,6 @@ describe Service do api_key: '123456789' }) end - let(:project) { create(:project) } describe 'is prefilled for projects pushover service' do it "has all fields prefilled" do |