diff options
64 files changed, 1018 insertions, 172 deletions
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index c0456c18e44..952963e0711 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -192,8 +192,9 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => { }); }; -export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { +export const saveDiffDiscussion = ({ state, dispatch }, { note, formData }) => { const postData = getNoteFormData({ + commit: state.commit, note, ...formData, }); diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 5c6b20109bb..cbaa0e26395 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -27,6 +27,7 @@ export const getReversePosition = linePosition => { export function getFormData(params) { const { + commit, note, noteableType, noteableData, @@ -66,7 +67,7 @@ export function getFormData(params) { position, noteable_type: noteableType, noteable_id: noteableData.id, - commit_id: '', + commit_id: commit && commit.id, type: diffFile.diff_refs.start_sha && diffFile.diff_refs.head_sha ? DIFF_NOTE_TYPE diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index d32f39881dd..75c18a9b6a0 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -155,7 +155,7 @@ export default class MilestoneSelect { const { $el, e } = clickEvent; let selected = clickEvent.selectedObj; - let data, boardsStore; + let data, modalStoreFilter; if (!selected) return; if (options.handleClick) { @@ -179,11 +179,11 @@ export default class MilestoneSelect { } if ($dropdown.closest('.add-issues-modal').length) { - boardsStore = ModalStore.store.filter; + modalStoreFilter = ModalStore.store.filter; } - if (boardsStore) { - boardsStore[$dropdown.data('fieldName')] = selected.name; + if (modalStoreFilter) { + modalStoreFilter[$dropdown.data('fieldName')] = selected.name; e.preventDefault(); } else if ($dropdown.hasClass('js-filter-submit') && (isIssueIndex || isMRIndex)) { return Issuable.filterResults($dropdown.closest('form')); diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select/dropdown_value_collapsed.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select/dropdown_value_collapsed.vue index e742900dbcb..373794fb1f2 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select/dropdown_value_collapsed.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select/dropdown_value_collapsed.vue @@ -44,6 +44,7 @@ export default { class="sidebar-collapsed-icon" data-placement="left" data-container="body" + data-boundary="viewport" @click="handleClick" > <i aria-hidden="true" data-hidden="true" class="fa fa-tags"> </i> diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 9aa98e2ca1f..a597996a362 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -102,7 +102,7 @@ module IssuableCollections elsif @group options[:group_id] = @group.id options[:include_subgroups] = true - options[:use_cte_for_search] = true + options[:attempt_group_search_optimizations] = true end params.permit(finder_type.valid_params).merge(options) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index e04e3a2a7e0..b73a3fa6e01 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -27,12 +27,13 @@ # created_before: datetime # updated_after: datetime # updated_before: datetime -# use_cte_for_search: boolean +# attempt_group_search_optimizations: boolean # class IssuableFinder prepend FinderWithCrossProjectAccess include FinderMethods include CreatedAtFilter + include Gitlab::Utils::StrongMemoize requires_cross_project_access unless: -> { project? } @@ -75,8 +76,9 @@ class IssuableFinder items = init_collection items = filter_items(items) - # This has to be last as we may use a CTE as an optimization fence by - # passing the use_cte_for_search param + # This has to be last as we may use a CTE as an optimization fence + # by passing the attempt_group_search_optimizations param and + # enabling the use_cte_for_group_issues_search feature flag # https://www.postgresql.org/docs/current/static/queries-with.html items = by_search(items) @@ -85,6 +87,8 @@ class IssuableFinder def filter_items(items) items = by_project(items) + items = by_group(items) + items = by_subquery(items) items = by_scope(items) items = by_created_at(items) items = by_updated_at(items) @@ -282,12 +286,31 @@ class IssuableFinder end # rubocop: enable CodeReuse/ActiveRecord + def use_subquery_for_search? + strong_memoize(:use_subquery_for_search) do + attempt_group_search_optimizations? && + Feature.enabled?(:use_subquery_for_group_issues_search, default_enabled: false) + end + end + + def use_cte_for_search? + strong_memoize(:use_cte_for_search) do + attempt_group_search_optimizations? && + !use_subquery_for_search? && + Feature.enabled?(:use_cte_for_group_issues_search, default_enabled: true) + end + end + private def init_collection klass.all end + def attempt_group_search_optimizations? + search && Gitlab::Database.postgresql? && params[:attempt_group_search_optimizations] + end + def count_key(value) Array(value).last.to_sym end @@ -351,12 +374,13 @@ class IssuableFinder end # rubocop: enable CodeReuse/ActiveRecord - def use_cte_for_search? - return false unless search - return false unless Gitlab::Database.postgresql? - return false unless Feature.enabled?(:use_cte_for_group_issues_search, default_enabled: true) - - params[:use_cte_for_search] + # Wrap projects and groups in a subquery if the conditions are met. + def by_subquery(items) + if use_subquery_for_search? + klass.where(id: items.select(:id)) # rubocop: disable CodeReuse/ActiveRecord + else + items + end end # rubocop: disable CodeReuse/ActiveRecord diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d60861dc95f..d86a6eceb59 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -120,7 +120,7 @@ module Ci acts_as_taggable - add_authentication_token_field :token + add_authentication_token_field :token, encrypted: true, fallback: true before_save :update_artifacts_size, if: :artifacts_file_changed? before_save :ensure_token diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index 67746e34913..c931b340b24 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -3,7 +3,7 @@ module Clusters module Applications class Runner < ActiveRecord::Base - VERSION = '0.1.38'.freeze + VERSION = '0.1.39'.freeze self.table_name = 'clusters_applications_runners' diff --git a/app/models/commit.rb b/app/models/commit.rb index 2c89da88b9b..a422a0995ff 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -177,7 +177,9 @@ class Commit def title return full_title if full_title.length < 100 - full_title.truncate(81, separator: ' ', omission: '…') + # Use three dots instead of the ellipsis Unicode character because + # some clients show the raw Unicode value in the merge commit. + full_title.truncate(81, separator: ' ', omission: '...') end # Returns the full commits title diff --git a/app/models/concerns/fast_destroy_all.rb b/app/models/concerns/fast_destroy_all.rb index 2bfa7da6c1c..1e3afd641ed 100644 --- a/app/models/concerns/fast_destroy_all.rb +++ b/app/models/concerns/fast_destroy_all.rb @@ -70,13 +70,14 @@ module FastDestroyAll module Helpers extend ActiveSupport::Concern + include AfterCommitQueue class_methods do ## # This method is to be defined on models which have fast destroyable models as children, # and let us avoid to use `dependent: :destroy` hook - def use_fast_destroy(relation) - before_destroy(prepend: true) do + def use_fast_destroy(relation, opts = {}) + set_callback :destroy, :before, opts.merge(prepend: true) do perform_fast_destroy(public_send(relation)) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/models/concerns/with_uploads.rb b/app/models/concerns/with_uploads.rb index 2bdef2a40e4..d79c0eae77e 100644 --- a/app/models/concerns/with_uploads.rb +++ b/app/models/concerns/with_uploads.rb @@ -17,6 +17,8 @@ module WithUploads extend ActiveSupport::Concern + include FastDestroyAll::Helpers + include FeatureGate # Currently there is no simple way how to select only not-mounted # uploads, it should be all FileUploaders so we select them by @@ -25,21 +27,40 @@ module WithUploads included do has_many :uploads, as: :model + has_many :file_uploads, -> { where(uploader: FILE_UPLOADERS) }, class_name: 'Upload', as: :model - before_destroy :destroy_file_uploads + # TODO: when feature flag is removed, we can use just dependent: destroy + # option on :file_uploads + before_destroy :remove_file_uploads + + use_fast_destroy :file_uploads, if: :fast_destroy_enabled? + end + + def retrieve_upload(_identifier, paths) + uploads.find_by(path: paths) end + private + # mounted uploads are deleted in carrierwave's after_commit hook, # but FileUploaders which are not mounted must be deleted explicitly and # it can not be done in after_commit because FileUploader requires loads # associated model on destroy (which is already deleted in after_commit) - def destroy_file_uploads - self.uploads.where(uploader: FILE_UPLOADERS).find_each do |upload| + def remove_file_uploads + fast_destroy_enabled? ? delete_uploads : destroy_uploads + end + + def delete_uploads + file_uploads.delete_all(:delete_all) + end + + def destroy_uploads + file_uploads.find_each do |upload| upload.destroy end end - def retrieve_upload(_identifier, paths) - uploads.find_by(path: paths) + def fast_destroy_enabled? + Feature.enabled?(:fast_destroy_uploads, self) end end diff --git a/app/models/shard.rb b/app/models/shard.rb index 2e75bc91df0..e39d4232486 100644 --- a/app/models/shard.rb +++ b/app/models/shard.rb @@ -18,7 +18,9 @@ class Shard < ActiveRecord::Base end def self.by_name(name) - find_or_create_by(name: name) + transaction(requires_new: true) do + find_or_create_by(name: name) + end rescue ActiveRecord::RecordNotUnique retry end diff --git a/app/models/upload.rb b/app/models/upload.rb index e01e9c6a4f0..20860f14b83 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -25,6 +25,25 @@ class Upload < ActiveRecord::Base Digest::SHA256.file(path).hexdigest end + class << self + ## + # FastDestroyAll concerns + def begin_fast_destroy + { + Uploads::Local => Uploads::Local.new.keys(with_files_stored_locally), + Uploads::Fog => Uploads::Fog.new.keys(with_files_stored_remotely) + } + end + + ## + # FastDestroyAll concerns + def finalize_fast_destroy(keys) + keys.each do |store_class, paths| + store_class.new.delete_keys_async(paths) + end + end + end + def absolute_path raise ObjectStorage::RemoteStoreError, "Remote object has no absolute path." unless local? return path unless relative_path? diff --git a/app/models/uploads/base.rb b/app/models/uploads/base.rb new file mode 100644 index 00000000000..f9814159958 --- /dev/null +++ b/app/models/uploads/base.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Uploads + class Base + BATCH_SIZE = 100 + + attr_reader :logger + + def initialize(logger: nil) + @logger ||= Rails.logger + end + + def delete_keys_async(keys_to_delete) + keys_to_delete.each_slice(BATCH_SIZE) do |batch| + DeleteStoredFilesWorker.perform_async(self.class, batch) + end + end + end +end diff --git a/app/models/uploads/fog.rb b/app/models/uploads/fog.rb new file mode 100644 index 00000000000..b44e273e9ab --- /dev/null +++ b/app/models/uploads/fog.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Uploads + class Fog < Base + include ::Gitlab::Utils::StrongMemoize + + def available? + object_store.enabled + end + + def keys(relation) + return [] unless available? + + relation.pluck(:path) + end + + def delete_keys(keys) + keys.each do |key| + connection.delete_object(bucket_name, key) + end + end + + private + + def object_store + Gitlab.config.uploads.object_store + end + + def bucket_name + return unless available? + + object_store.remote_directory + end + + def connection + return unless available? + + strong_memoize(:connection) do + ::Fog::Storage.new(object_store.connection.to_hash.deep_symbolize_keys) + end + end + end +end diff --git a/app/models/uploads/local.rb b/app/models/uploads/local.rb new file mode 100644 index 00000000000..2901c33c359 --- /dev/null +++ b/app/models/uploads/local.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module Uploads + class Local < Base + def keys(relation) + relation.includes(:model).find_each.map(&:absolute_path) + end + + def delete_keys(keys) + keys.each do |path| + delete_file(path) + end + end + + private + + def delete_file(path) + unless exists?(path) + logger.warn("File '#{path}' doesn't exist, skipping") + return + end + + unless in_uploads?(path) + message = "Path '#{path}' is not in uploads dir, skipping" + logger.warn(message) + Gitlab::Sentry.track_exception(RuntimeError.new(message), extra: { uploads_dir: storage_dir }) + return + end + + FileUtils.rm(path) + delete_dir!(File.dirname(path)) + end + + def exists?(path) + path.present? && File.exist?(path) + end + + def in_uploads?(path) + path.start_with?(storage_dir) + end + + def delete_dir!(path) + Dir.rmdir(path) + rescue Errno::ENOENT + # Ignore: path does not exist + rescue Errno::ENOTDIR + # Ignore: path is not a dir + rescue Errno::ENOTEMPTY, Errno::EEXIST + # Ignore: dir is not empty + end + + def storage_dir + @storage_dir ||= File.realpath(Gitlab.config.uploads.storage_path) + end + end +end diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index 2c6484c2c99..56b06374d6d 100644 --- a/app/views/projects/labels/index.html.haml +++ b/app/views/projects/labels/index.html.haml @@ -5,7 +5,7 @@ - subscribed = params[:subscribed] - labels_or_filters = @labels.exists? || @prioritized_labels.exists? || search.present? || subscribed.present? -- if @labels.present? && can_admin_label +- if labels_or_filters && can_admin_label - content_for(:header_content) do .nav-controls = link_to _('New label'), new_project_label_path(@project), class: "btn btn-success qa-label-create-new" diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 2c55806a286..d9fd395c5ec 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -134,3 +134,4 @@ - delete_diff_files - detect_repository_languages - repository_cleanup +- delete_stored_files diff --git a/app/workers/delete_stored_files_worker.rb b/app/workers/delete_stored_files_worker.rb new file mode 100644 index 00000000000..ff7931849d8 --- /dev/null +++ b/app/workers/delete_stored_files_worker.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class DeleteStoredFilesWorker + include ApplicationWorker + + def perform(class_name, keys) + klass = begin + class_name.constantize + rescue NameError + nil + end + + unless klass + message = "Unknown class '#{class_name}'" + logger.error(message) + Gitlab::Sentry.track_exception(RuntimeError.new(message)) + return + end + + klass.new(logger: logger).delete_keys(keys) + end +end diff --git a/changelogs/unreleased/fix-gb-encrypt-ci-build-token.yml b/changelogs/unreleased/fix-gb-encrypt-ci-build-token.yml new file mode 100644 index 00000000000..04fc88bc3d3 --- /dev/null +++ b/changelogs/unreleased/fix-gb-encrypt-ci-build-token.yml @@ -0,0 +1,5 @@ +--- +title: Encrypt CI/CD builds authentication tokens +merge_request: 23436 +author: +type: security diff --git a/changelogs/unreleased/gt-show-primary-button-when-all-labels-are-prioritized.yml b/changelogs/unreleased/gt-show-primary-button-when-all-labels-are-prioritized.yml new file mode 100644 index 00000000000..eed31950a76 --- /dev/null +++ b/changelogs/unreleased/gt-show-primary-button-when-all-labels-are-prioritized.yml @@ -0,0 +1,5 @@ +--- +title: Show primary button when all labels are prioritized +merge_request: 23648 +author: George Tsiolis +type: other diff --git a/changelogs/unreleased/move-group-issues-search-cte-up-the-chain.yml b/changelogs/unreleased/move-group-issues-search-cte-up-the-chain.yml new file mode 100644 index 00000000000..0269e7b6196 --- /dev/null +++ b/changelogs/unreleased/move-group-issues-search-cte-up-the-chain.yml @@ -0,0 +1,5 @@ +--- +title: Fix error when searching for group issues with priority or popularity sort +merge_request: 23445 +author: +type: fixed diff --git a/changelogs/unreleased/sh-truncate-with-periods.yml b/changelogs/unreleased/sh-truncate-with-periods.yml new file mode 100644 index 00000000000..b1c6b4f9cbd --- /dev/null +++ b/changelogs/unreleased/sh-truncate-with-periods.yml @@ -0,0 +1,5 @@ +--- +title: Truncate merge request titles with periods instead of ellipsis +merge_request: 23558 +author: +type: changed diff --git a/changelogs/unreleased/update-gitlab-runner-helm-chart-to-0-1-39.yml b/changelogs/unreleased/update-gitlab-runner-helm-chart-to-0-1-39.yml new file mode 100644 index 00000000000..dffcdb0bb5a --- /dev/null +++ b/changelogs/unreleased/update-gitlab-runner-helm-chart-to-0-1-39.yml @@ -0,0 +1,5 @@ +--- +title: Update used version of Runner Helm Chart to 0.1.39 +merge_request: 23633 +author: +type: other diff --git a/changelogs/unreleased/winh-merge-request-diff-discussion-commit-id.yml b/changelogs/unreleased/winh-merge-request-diff-discussion-commit-id.yml new file mode 100644 index 00000000000..2ce16a2b6b7 --- /dev/null +++ b/changelogs/unreleased/winh-merge-request-diff-discussion-commit-id.yml @@ -0,0 +1,5 @@ +--- +title: Pass commit when posting diff discussions +merge_request: 23371 +author: +type: fixed diff --git a/changelogs/unreleased/winh-milestone-select.yml b/changelogs/unreleased/winh-milestone-select.yml new file mode 100644 index 00000000000..8464fc6c541 --- /dev/null +++ b/changelogs/unreleased/winh-milestone-select.yml @@ -0,0 +1,5 @@ +--- +title: Fix milestone select in issue sidebar of issue boards +merge_request: 23625 +author: +type: fixed diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index d8002815bac..4782a223561 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -82,3 +82,4 @@ - [detect_repository_languages, 1] - [auto_devops, 2] - [repository_cleanup, 1] + - [delete_stored_files, 1] diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile index 87c61d6e90d..be7b301866d 100644 --- a/danger/documentation/Dangerfile +++ b/danger/documentation/Dangerfile @@ -24,23 +24,24 @@ The following files require a review from the Documentation team: * #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")} -When your content is ready for review, mention a technical writer in a separate -comment and explain what needs to be reviewed. - -You are welcome to mention them sooner if you have questions about writing or updating -the documentation. GitLabbers are also welcome to use the [#docs](https://gitlab.slack.com/archives/C16HYA2P5) channel on Slack. - -Who to ping [based on DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages): +When your content is ready for review, assign the MR to a technical writer +according to the [DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages) +in the table below. If necessary, mention them in a comment explaining what needs +to be reviewed. | Tech writer | Stage(s) | | ------------ | ------------------------------------------------------------ | -| `@marcia` | ~Create ~Release | +| `@marcia` | ~Create ~Release + ~"development guidelines" | | `@axil` | ~Distribution ~Gitaly ~Gitter ~Monitoring ~Packaging ~Secure | | `@eread` | ~Manage ~Configure ~Geo ~Verify | | `@mikelewis` | ~Plan | +You are welcome to mention them sooner if you have questions about writing or +updating the documentation. GitLabbers are also welcome to use the +[#docs](https://gitlab.slack.com/archives/C16HYA2P5) channel on Slack. + If you are not sure which category the change falls within, or the change is not -part of one of these categories, you can mention one of the usernames above. +part of one of these categories, mention one of the usernames above. MARKDOWN unless gitlab.mr_labels.include?('Documentation') diff --git a/db/fixtures/development/04_project.rb b/db/fixtures/development/04_project.rb index 089de211380..aa8686ac7d8 100644 --- a/db/fixtures/development/04_project.rb +++ b/db/fixtures/development/04_project.rb @@ -71,13 +71,17 @@ Sidekiq::Testing.inline! do params[:storage_version] = Project::LATEST_STORAGE_VERSION end - project = Projects::CreateService.new(User.first, params).execute - # Seed-Fu runs this entire fixture in a transaction, so the `after_commit` - # hook won't run until after the fixture is loaded. That is too late - # since the Sidekiq::Testing block has already exited. Force clearing - # the `after_commit` queue to ensure the job is run now. + project = nil + Sidekiq::Worker.skipping_transaction_check do + project = Projects::CreateService.new(User.first, params).execute + + # Seed-Fu runs this entire fixture in a transaction, so the `after_commit` + # hook won't run until after the fixture is loaded. That is too late + # since the Sidekiq::Testing block has already exited. Force clearing + # the `after_commit` queue to ensure the job is run now. project.send(:_run_after_commit_queue) + project.import_state.send(:_run_after_commit_queue) end if project.valid? && project.valid_repo? diff --git a/db/fixtures/development/10_merge_requests.rb b/db/fixtures/development/10_merge_requests.rb index bcfdd058a1c..8bdc7c6556c 100644 --- a/db/fixtures/development/10_merge_requests.rb +++ b/db/fixtures/development/10_merge_requests.rb @@ -25,7 +25,9 @@ Gitlab::Seeder.quiet do developer = project.team.developers.sample break unless developer - MergeRequests::CreateService.new(project, developer, params).execute + Sidekiq::Worker.skipping_transaction_check do + MergeRequests::CreateService.new(project, developer, params).execute + end print '.' end end @@ -39,7 +41,9 @@ Gitlab::Seeder.quiet do target_branch: 'master', title: 'Can be automatically merged' } - MergeRequests::CreateService.new(project, User.admins.first, params).execute + Sidekiq::Worker.skipping_transaction_check do + MergeRequests::CreateService.new(project, User.admins.first, params).execute + end print '.' params = { @@ -47,6 +51,8 @@ Gitlab::Seeder.quiet do target_branch: 'feature', title: 'Cannot be automatically merged' } - MergeRequests::CreateService.new(project, User.admins.first, params).execute + Sidekiq::Worker.skipping_transaction_check do + MergeRequests::CreateService.new(project, User.admins.first, params).execute + end print '.' end diff --git a/db/migrate/20181129104854_add_token_encrypted_to_ci_builds.rb b/db/migrate/20181129104854_add_token_encrypted_to_ci_builds.rb new file mode 100644 index 00000000000..11b98203793 --- /dev/null +++ b/db/migrate/20181129104854_add_token_encrypted_to_ci_builds.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddTokenEncryptedToCiBuilds < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_builds, :token_encrypted, :string + end +end diff --git a/db/migrate/20181129104944_add_index_to_ci_builds_token_encrypted.rb b/db/migrate/20181129104944_add_index_to_ci_builds_token_encrypted.rb new file mode 100644 index 00000000000..f90aca008e5 --- /dev/null +++ b/db/migrate/20181129104944_add_index_to_ci_builds_token_encrypted.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexToCiBuildsTokenEncrypted < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_builds, :token_encrypted, unique: true, where: 'token_encrypted IS NOT NULL' + end + + def down + remove_concurrent_index :ci_builds, :token_encrypted + end +end diff --git a/db/schema.rb b/db/schema.rb index fc73d30fb1f..d7124100621 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -345,6 +345,7 @@ ActiveRecord::Schema.define(version: 20181203002526) do t.boolean "protected" t.integer "failure_reason" t.datetime_with_timezone "scheduled_at" + t.string "token_encrypted" t.index ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)", using: :btree t.index ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree t.index ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree @@ -361,6 +362,7 @@ ActiveRecord::Schema.define(version: 20181203002526) do t.index ["stage_id"], name: "index_ci_builds_on_stage_id", using: :btree t.index ["status", "type", "runner_id"], name: "index_ci_builds_on_status_and_type_and_runner_id", using: :btree t.index ["token"], name: "index_ci_builds_on_token", unique: true, using: :btree + t.index ["token_encrypted"], name: "index_ci_builds_on_token_encrypted", unique: true, where: "(token_encrypted IS NOT NULL)", using: :btree t.index ["updated_at"], name: "index_ci_builds_on_updated_at", using: :btree t.index ["user_id"], name: "index_ci_builds_on_user_id", using: :btree end diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index fc03cf6cc39..9ff6c73b1b6 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -974,10 +974,9 @@ curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://git Merge changes submitted with MR using this API. +If merge request is unable to be accepted (ie: Work in Progress, Closed, Pipeline Pending Completion, or Failed while requiring Success) - you'll get a `405` and the error message 'Method Not Allowed' -If it has some conflicts and can not be merged - you'll get a `405` and the error message 'Branch cannot be merged' - -If merge request is already merged or closed - you'll get a `406` and the error message 'Method Not Allowed' +If it has some conflicts and can not be merged - you'll get a `406` and the error message 'Branch cannot be merged' If the `sha` parameter is passed and does not match the HEAD of the source - you'll get a `409` and the error message 'SHA does not match HEAD of source branch' diff --git a/doc/api/search.md b/doc/api/search.md index a9369930003..7e3ae7404a3 100644 --- a/doc/api/search.md +++ b/doc/api/search.md @@ -722,16 +722,22 @@ Example response: ### Scope: wiki_blobs +Filters are available for this scope: + +- filename +- path +- extension + +To use a filter simply include it in your query like: `a query filename:some_name*`. +You may use wildcards (`*`) to use glob matching. + Wiki blobs searches are performed on both filenames and contents. Search results: - Found in filenames are displayed before results found in contents. - May contain multiple matches for the same blob because the search string - might be found in both the filename and content, and matches of the different -types are displayed separately. -- May contain multiple matches for the same blob because the search string - might be found if the search string appears multiple times in the content. - + might be found in both the filename and content, or might appear multiple + times in the content. ```bash curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/6/search?scope=wiki_blobs&search=bye @@ -788,22 +794,20 @@ Example response: ### Scope: blobs Filters are available for this scope: + - filename - path - extension -to use a filter simply include it in your query like so: `a query filename:some_name*`. +To use a filter simply include it in your query like: `a query filename:some_name*`. +You may use wildcards (`*`) to use glob matching. Blobs searches are performed on both filenames and contents. Search results: - Found in filenames are displayed before results found in contents. - May contain multiple matches for the same blob because the search string - might be found in both the filename and content, and matches of the different -types are displayed separately. -- May contain multiple matches for the same blob because the search string - might be found if the search string appears multiple times in the content. - -You may use wildcards (`*`) to use glob matching. + might be found in both the filename and content, or might appear multiple + times in the content. ```bash curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/6/search?scope=blobs&search=installation diff --git a/doc/ci/README.md b/doc/ci/README.md index dba1f38abe2..4e066a0df97 100644 --- a/doc/ci/README.md +++ b/doc/ci/README.md @@ -71,6 +71,7 @@ learn how to leverage its potential even more. - [Caching dependencies](caching/index.md) - [Git submodules](git_submodules.md) - How to run your CI jobs when Git submodules are involved +- [Pipelines for merge requests](merge_request_pipelines/index.md) - [Use SSH keys in your build environment](ssh_keys/README.md) - [Trigger pipelines through the GitLab API](triggers/README.md) - [Trigger pipelines on a schedule](../user/project/pipelines/schedules.md) diff --git a/doc/ci/merge_request_pipelines/img/merge_request.png b/doc/ci/merge_request_pipelines/img/merge_request.png Binary files differnew file mode 100644 index 00000000000..1fe2eec2008 --- /dev/null +++ b/doc/ci/merge_request_pipelines/img/merge_request.png diff --git a/doc/ci/merge_request_pipelines/img/pipeline_detail.png b/doc/ci/merge_request_pipelines/img/pipeline_detail.png Binary files differnew file mode 100644 index 00000000000..def1781dd75 --- /dev/null +++ b/doc/ci/merge_request_pipelines/img/pipeline_detail.png diff --git a/doc/ci/merge_request_pipelines/index.md b/doc/ci/merge_request_pipelines/index.md new file mode 100644 index 00000000000..706e83abf44 --- /dev/null +++ b/doc/ci/merge_request_pipelines/index.md @@ -0,0 +1,84 @@ +# Pipelines for merge requests + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/15310) in GitLab 11.6 + +Usually, when a developer creates a new merge request, a pipeline runs on the +new change and checks if it's qualified to be merged into a target branch. This +pipeline should contain only necessary jobs for checking the new changes. +For example, unit tests, lint checks, and Review Apps are often used in this cycle. + +With pipelines for merge requests, you can design a specific pipeline structure +for merge requests. All you need to do is just adding `only: [merge_requests]` to +the jobs that you want it to run for only merge requests. +Every time, when developers create or update merge requests, a pipeline runs on +their new commits at every push to GitLab. + +NOTE: **Note**: +If you use both this feature and the [Merge When Pipeline Succeeds](../../user/project/merge_requests/merge_when_pipeline_succeeds.md) +feature, pipelines for merge requests take precendence over the other regular pipelines. + +For example, consider a GitLab CI/CD configuration in .gitlab-ci.yml as follows: + +```yaml +build: + stage: build + script: ./build + only: + - branches + - tags + - merge_requests + +test: + stage: test + script: ./test + only: + - merge_requests + +deploy: + stage: deploy + script: ./deploy +``` + +After a developer updated code in a merge request with whatever methods (e.g. `git push`), +GitLab detects that the code is updated and create a new pipeline for the merge request. +The pipeline fetches the latest code from the source branch and run tests against it. +In this example, the pipeline contains only `build` and `test` jobs. +Since `deploy` job does not have the `only: [merge_requests]` rule, +deployment jobs will not happen in the merge request. + +Consider this pipeline list viewed from the **Pipelines** tab in a merge request: + + + +Note that pipelines tagged as **merge request** indicate that they were triggered +when a merge request was created or updated. + +The same tag is shown on the pipeline's details: + + + +## Important notes about merge requests from forked projects + +Note that the current behavior is subject to change. In the usual contribution +flow, external contributors follow the following steps: + +1. Fork a parent project. +1. Create a merge request from the forked project that targets the `master` branch +in the parent project. +1. A pipeline runs on the merge request. +1. A mainatiner from the parent project checks the pipeline result, and merge +into a target branch if the latest pipeline has passed. + +Currently, those pipelines are created in a **forked** project, not in the +parent project. This means you cannot completely trust the pipeline result, +because, technically, external contributors can disguise their pipeline results +by tweaking their GitLab Runner in the forked project. + +There are multiple reasons about why GitLab doesn't allow those pipelines to be +created in the parent project, but one of the biggest reasons is security concern. +External users could steal secret variables from the parent project by modifying +.gitlab-ci.yml, which could be some sort of credentials. This should not happen. + +We're discussing a secure solution of running pipelines for merge requests +that submitted from forked projects, +see [the issue about the permission extension](https://gitlab.com/gitlab-org/gitlab-ce/issues/23902). diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index fd81a67dca0..87799be8ab4 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -40,73 +40,84 @@ Starting with GitLab 9.0, we have deprecated some variables. Read the strongly advised to use the new variables as we will remove the old ones in future GitLab releases.** -| Variable | GitLab | Runner | Description | -|-------------------------------- |--------|--------|-------------| -| **ARTIFACT_DOWNLOAD_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to download artifacts running a job | -| **CI** | all | 0.4 | Mark that job is executed in CI environment | -| **CI_COMMIT_REF_NAME** | 9.0 | all | The branch or tag name for which project is built | -| **CI_COMMIT_REF_SLUG** | 9.0 | all | `$CI_COMMIT_REF_NAME` lowercased, shortened to 63 bytes, and with everything except `0-9` and `a-z` replaced with `-`. No leading / trailing `-`. Use in URLs, host names and domain names. | -| **CI_COMMIT_SHA** | 9.0 | all | The commit revision for which project is built | -| **CI_COMMIT_BEFORE_SHA** | 11.2 | all | The previous latest commit present on a branch before a push request. | -| **CI_COMMIT_TAG** | 9.0 | 0.5 | The commit tag name. Present only when building tags. | -| **CI_COMMIT_MESSAGE** | 10.8 | all | The full commit message. | -| **CI_COMMIT_TITLE** | 10.8 | all | The title of the commit - the full first line of the message | -| **CI_COMMIT_DESCRIPTION** | 10.8 | all | The description of the commit: the message without first line, if the title is shorter than 100 characters; full message in other case. | -| **CI_CONFIG_PATH** | 9.4 | 0.5 | The path to CI config file. Defaults to `.gitlab-ci.yml` | -| **CI_DEBUG_TRACE** | all | 1.7 | Whether [debug tracing](#debug-tracing) is enabled | -| **CI_DEPLOY_USER** | 10.8 | all | Authentication username of the [GitLab Deploy Token][gitlab-deploy-token], only present if the Project has one related.| -| **CI_DEPLOY_PASSWORD** | 10.8 | all | Authentication password of the [GitLab Deploy Token][gitlab-deploy-token], only present if the Project has one related.| -| **CI_DISPOSABLE_ENVIRONMENT** | all | 10.1 | Marks that the job is executed in a disposable environment (something that is created only for this job and disposed of/destroyed after the execution - all executors except `shell` and `ssh`). If the environment is disposable, it is set to true, otherwise it is not defined at all. | -| **CI_ENVIRONMENT_NAME** | 8.15 | all | The name of the environment for this job | -| **CI_ENVIRONMENT_SLUG** | 8.15 | all | A simplified version of the environment name, suitable for inclusion in DNS, URLs, Kubernetes labels, etc. | -| **CI_ENVIRONMENT_URL** | 9.3 | all | The URL of the environment for this job | -| **CI_JOB_ID** | 9.0 | all | The unique id of the current job that GitLab CI uses internally | -| **CI_JOB_MANUAL** | 8.12 | all | The flag to indicate that job was manually started | -| **CI_JOB_NAME** | 9.0 | 0.5 | The name of the job as defined in `.gitlab-ci.yml` | -| **CI_JOB_STAGE** | 9.0 | 0.5 | The name of the stage as defined in `.gitlab-ci.yml` | -| **CI_JOB_TOKEN** | 9.0 | 1.2 | Token used for authenticating with the [GitLab Container Registry][registry] and downloading [dependent repositories][dependent-repositories] | -| **CI_NODE_INDEX** | 11.5 | all | Index of the job in the job set. If the job is not parallelized, this variable is not set. | -| **CI_NODE_TOTAL** | 11.5 | all | Total number of instances of this job running in parallel. If the job is not parallelized, this variable is set to `1`. | -| **CI_JOB_URL** | 11.1 | 0.5 | Job details URL | -| **CI_REPOSITORY_URL** | 9.0 | all | The URL to clone the Git repository | -| **CI_RUNNER_DESCRIPTION** | 8.10 | 0.5 | The description of the runner as saved in GitLab | -| **CI_RUNNER_ID** | 8.10 | 0.5 | The unique id of runner being used | -| **CI_RUNNER_TAGS** | 8.10 | 0.5 | The defined runner tags | -| **CI_RUNNER_VERSION** | all | 10.6 | GitLab Runner version that is executing the current job | -| **CI_RUNNER_REVISION** | all | 10.6 | GitLab Runner revision that is executing the current job | -| **CI_RUNNER_EXECUTABLE_ARCH** | all | 10.6 | The OS/architecture of the GitLab Runner executable (note that this is not necessarily the same as the environment of the executor) | -| **CI_PIPELINE_ID** | 8.10 | 0.5 | The unique id of the current pipeline that GitLab CI uses internally | -| **CI_PIPELINE_IID** | 11.0 | all | The unique id of the current pipeline scoped to project | -| **CI_PIPELINE_TRIGGERED** | all | all | The flag to indicate that job was [triggered] | -| **CI_PIPELINE_SOURCE** | 10.0 | all | Indicates how the pipeline was triggered. Possible options are: `push`, `web`, `trigger`, `schedule`, `api`, and `pipeline`. For pipelines created before GitLab 9.5, this will show as `unknown` | -| **CI_PROJECT_DIR** | all | all | The full path where the repository is cloned and where the job is run | -| **CI_PROJECT_ID** | all | all | The unique id of the current project that GitLab CI uses internally | -| **CI_PROJECT_NAME** | 8.10 | 0.5 | The project name that is currently being built (actually it is project folder name) | -| **CI_PROJECT_NAMESPACE** | 8.10 | 0.5 | The project namespace (username or groupname) that is currently being built | -| **CI_PROJECT_PATH** | 8.10 | 0.5 | The namespace with project name | -| **CI_PROJECT_PATH_SLUG** | 9.3 | all | `$CI_PROJECT_PATH` lowercased and with everything except `0-9` and `a-z` replaced with `-`. Use in URLs and domain names. | -| **CI_PIPELINE_URL** | 11.1 | 0.5 | Pipeline details URL | -| **CI_PROJECT_URL** | 8.10 | 0.5 | The HTTP address to access project | -| **CI_PROJECT_VISIBILITY** | 10.3 | all | The project visibility (internal, private, public) | -| **CI_REGISTRY** | 8.10 | 0.5 | If the Container Registry is enabled it returns the address of GitLab's Container Registry | -| **CI_REGISTRY_IMAGE** | 8.10 | 0.5 | If the Container Registry is enabled for the project it returns the address of the registry tied to the specific project | -| **CI_REGISTRY_PASSWORD** | 9.0 | all | The password to use to push containers to the GitLab Container Registry | -| **CI_REGISTRY_USER** | 9.0 | all | The username to use to push containers to the GitLab Container Registry | -| **CI_SERVER** | all | all | Mark that job is executed in CI environment | -| **CI_SERVER_NAME** | all | all | The name of CI server that is used to coordinate jobs | -| **CI_SERVER_REVISION** | all | all | GitLab revision that is used to schedule jobs | -| **CI_SERVER_VERSION** | all | all | GitLab version that is used to schedule jobs | -| **CI_SERVER_VERSION_MAJOR** | 11.4 | all | GitLab version major component | -| **CI_SERVER_VERSION_MINOR** | 11.4 | all | GitLab version minor component | -| **CI_SERVER_VERSION_PATCH** | 11.4 | all | GitLab version patch component | -| **CI_SHARED_ENVIRONMENT** | all | 10.1 | Marks that the job is executed in a shared environment (something that is persisted across CI invocations like `shell` or `ssh` executor). If the environment is shared, it is set to true, otherwise it is not defined at all. | -| **GET_SOURCES_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to fetch sources running a job | -| **GITLAB_CI** | all | all | Mark that job is executed in GitLab CI environment | -| **GITLAB_USER_EMAIL** | 8.12 | all | The email of the user who started the job | -| **GITLAB_USER_ID** | 8.12 | all | The id of the user who started the job | -| **GITLAB_USER_LOGIN** | 10.0 | all | The login username of the user who started the job | -| **GITLAB_USER_NAME** | 10.0 | all | The real name of the user who started the job | -| **RESTORE_CACHE_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to restore the cache running a job | +| Variable | GitLab | Runner | Description | +|-------------------------------------------|--------|--------|-------------| +| **ARTIFACT_DOWNLOAD_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to download artifacts running a job | +| **CI** | all | 0.4 | Mark that job is executed in CI environment | +| **CI_COMMIT_REF_NAME** | 9.0 | all | The branch or tag name for which project is built | +| **CI_COMMIT_REF_SLUG** | 9.0 | all | `$CI_COMMIT_REF_NAME` lowercased, shortened to 63 bytes, and with everything except `0-9` and `a-z` replaced with `-`. No leading / trailing `-`. Use in URLs, host names and domain names. | +| **CI_COMMIT_SHA** | 9.0 | all | The commit revision for which project is built | +| **CI_COMMIT_BEFORE_SHA** | 11.2 | all | The previous latest commit present on a branch before a push request. | +| **CI_COMMIT_TAG** | 9.0 | 0.5 | The commit tag name. Present only when building tags. | +| **CI_COMMIT_MESSAGE** | 10.8 | all | The full commit message. | +| **CI_COMMIT_TITLE** | 10.8 | all | The title of the commit - the full first line of the message | +| **CI_COMMIT_DESCRIPTION** | 10.8 | all | The description of the commit: the message without first line, if the title is shorter than 100 characters; full message in other case. | +| **CI_CONFIG_PATH** | 9.4 | 0.5 | The path to CI config file. Defaults to `.gitlab-ci.yml` | +| **CI_DEBUG_TRACE** | all | 1.7 | Whether [debug tracing](#debug-tracing) is enabled | +| **CI_DEPLOY_USER** | 10.8 | all | Authentication username of the [GitLab Deploy Token][gitlab-deploy-token], only present if the Project has one related.| +| **CI_DEPLOY_PASSWORD** | 10.8 | all | Authentication password of the [GitLab Deploy Token][gitlab-deploy-token], only present if the Project has one related.| +| **CI_DISPOSABLE_ENVIRONMENT** | all | 10.1 | Marks that the job is executed in a disposable environment (something that is created only for this job and disposed of/destroyed after the execution - all executors except `shell` and `ssh`). If the environment is disposable, it is set to true, otherwise it is not defined at all. | +| **CI_ENVIRONMENT_NAME** | 8.15 | all | The name of the environment for this job | +| **CI_ENVIRONMENT_SLUG** | 8.15 | all | A simplified version of the environment name, suitable for inclusion in DNS, URLs, Kubernetes labels, etc. | +| **CI_ENVIRONMENT_URL** | 9.3 | all | The URL of the environment for this job | +| **CI_JOB_ID** | 9.0 | all | The unique id of the current job that GitLab CI uses internally | +| **CI_JOB_MANUAL** | 8.12 | all | The flag to indicate that job was manually started | +| **CI_JOB_NAME** | 9.0 | 0.5 | The name of the job as defined in `.gitlab-ci.yml` | +| **CI_JOB_STAGE** | 9.0 | 0.5 | The name of the stage as defined in `.gitlab-ci.yml` | +| **CI_JOB_TOKEN** | 9.0 | 1.2 | Token used for authenticating with the [GitLab Container Registry][registry] and downloading [dependent repositories][dependent-repositories] | +| **CI_MERGE_REQUEST_ID** | 11.6 | all | The ID of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) | +| **CI_MERGE_REQUEST_IID** | 11.6 | all | The IID of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) | +| **CI_MERGE_REQUEST_REF_PATH** | 11.6 | all | The ref path of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md). (e.g. `refs/merge-requests/1/head`) | +| **CI_MERGE_REQUEST_PROJECT_ID** | 11.6 | all | The ID of the project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) | +| **CI_MERGE_REQUEST_PROJECT_PATH** | 11.6 | all | The path of the project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) (e.g. `namespace/awesome-project`) | +| **CI_MERGE_REQUEST_PROJECT_URL** | 11.6 | all | The URL of the project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) (e.g. `http://192.168.10.15:3000/namespace/awesome-project`) | +| **CI_MERGE_REQUEST_TARGET_BRANCH_NAME** | 11.6 | all | The target branch name of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) | +| **CI_MERGE_REQUEST_SOURCE_PROJECT_ID** | 11.6 | all | The ID of the source project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) | +| **CI_MERGE_REQUEST_SOURCE_PROJECT_PATH** | 11.6 | all | The path of the source project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) | +| **CI_MERGE_REQUEST_SOURCE_PROJECT_URL** | 11.6 | all | The URL of the source project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) | +| **CI_MERGE_REQUEST_SOURCE_BRANCH_NAME** | 11.6 | all | The source branch name of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) | +| **CI_NODE_INDEX** | 11.5 | all | Index of the job in the job set. If the job is not parallelized, this variable is not set. | +| **CI_NODE_TOTAL** | 11.5 | all | Total number of instances of this job running in parallel. If the job is not parallelized, this variable is set to `1`. | +| **CI_JOB_URL** | 11.1 | 0.5 | Job details URL | +| **CI_REPOSITORY_URL** | 9.0 | all | The URL to clone the Git repository | +| **CI_RUNNER_DESCRIPTION** | 8.10 | 0.5 | The description of the runner as saved in GitLab | +| **CI_RUNNER_ID** | 8.10 | 0.5 | The unique id of runner being used | +| **CI_RUNNER_TAGS** | 8.10 | 0.5 | The defined runner tags | +| **CI_RUNNER_VERSION** | all | 10.6 | GitLab Runner version that is executing the current job | +| **CI_RUNNER_REVISION** | all | 10.6 | GitLab Runner revision that is executing the current job | +| **CI_RUNNER_EXECUTABLE_ARCH** | all | 10.6 | The OS/architecture of the GitLab Runner executable (note that this is not necessarily the same as the environment of the executor) | +| **CI_PIPELINE_ID** | 8.10 | 0.5 | The unique id of the current pipeline that GitLab CI uses internally | +| **CI_PIPELINE_IID** | 11.0 | all | The unique id of the current pipeline scoped to project | +| **CI_PIPELINE_TRIGGERED** | all | all | The flag to indicate that job was [triggered] | +| **CI_PIPELINE_SOURCE** | 10.0 | all | Indicates how the pipeline was triggered. Possible options are: `push`, `web`, `trigger`, `schedule`, `api`, and `pipeline`. For pipelines created before GitLab 9.5, this will show as `unknown` | +| **CI_PROJECT_DIR** | all | all | The full path where the repository is cloned and where the job is run | +| **CI_PROJECT_ID** | all | all | The unique id of the current project that GitLab CI uses internally | +| **CI_PROJECT_NAME** | 8.10 | 0.5 | The project name that is currently being built (actually it is project folder name) | +| **CI_PROJECT_NAMESPACE** | 8.10 | 0.5 | The project namespace (username or groupname) that is currently being built | +| **CI_PROJECT_PATH** | 8.10 | 0.5 | The namespace with project name | +| **CI_PROJECT_PATH_SLUG** | 9.3 | all | `$CI_PROJECT_PATH` lowercased and with everything except `0-9` and `a-z` replaced with `-`. Use in URLs and domain names. | +| **CI_PIPELINE_URL** | 11.1 | 0.5 | Pipeline details URL | +| **CI_PROJECT_URL** | 8.10 | 0.5 | The HTTP address to access project | +| **CI_PROJECT_VISIBILITY** | 10.3 | all | The project visibility (internal, private, public) | +| **CI_REGISTRY** | 8.10 | 0.5 | If the Container Registry is enabled it returns the address of GitLab's Container Registry | +| **CI_REGISTRY_IMAGE** | 8.10 | 0.5 | If the Container Registry is enabled for the project it returns the address of the registry tied to the specific project | +| **CI_REGISTRY_PASSWORD** | 9.0 | all | The password to use to push containers to the GitLab Container Registry | +| **CI_REGISTRY_USER** | 9.0 | all | The username to use to push containers to the GitLab Container Registry | +| **CI_SERVER** | all | all | Mark that job is executed in CI environment | +| **CI_SERVER_NAME** | all | all | The name of CI server that is used to coordinate jobs | +| **CI_SERVER_REVISION** | all | all | GitLab revision that is used to schedule jobs | +| **CI_SERVER_VERSION** | all | all | GitLab version that is used to schedule jobs | +| **CI_SERVER_VERSION_MAJOR** | 11.4 | all | GitLab version major component | +| **CI_SERVER_VERSION_MINOR** | 11.4 | all | GitLab version minor component | +| **CI_SERVER_VERSION_PATCH** | 11.4 | all | GitLab version patch component | +| **CI_SHARED_ENVIRONMENT** | all | 10.1 | Marks that the job is executed in a shared environment (something that is persisted across CI invocations like `shell` or `ssh` executor). If the environment is shared, it is set to true, otherwise it is not defined at all. | +| **GET_SOURCES_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to fetch sources running a job | +| **GITLAB_CI** | all | all | Mark that job is executed in GitLab CI environment | +| **GITLAB_USER_EMAIL** | 8.12 | all | The email of the user who started the job | +| **GITLAB_USER_ID** | 8.12 | all | The id of the user who started the job | +| **GITLAB_USER_LOGIN** | 10.0 | all | The login username of the user who started the job | +| **GITLAB_USER_NAME** | 10.0 | all | The real name of the user who started the job | +| **RESTORE_CACHE_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to restore the cache running a job | ## GitLab 9.0 renaming diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index af7e41db443..1277d1fdf8b 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -342,15 +342,16 @@ In addition, `only` and `except` allow the use of special keywords: | **Value** | **Description** | | --------- | ---------------- | -| `branches` | When a branch is pushed. | -| `tags` | When a tag is pushed. | -| `api` | When pipeline has been triggered by a second pipelines API (not triggers API). | -| `external` | When using CI services other than GitLab. | -| `pipelines` | For multi-project triggers, created using the API with `CI_JOB_TOKEN`. | -| `pushes` | Pipeline is triggered by a `git push` by the user. | -| `schedules` | For [scheduled pipelines][schedules]. | -| `triggers` | For pipelines created using a trigger token. | -| `web` | For pipelines created using **Run pipeline** button in GitLab UI (under your project's **Pipelines**). | +| `branches` | When a git reference of a pipeline is a branch. | +| `tags` | When a git reference of a pipeline is a tag. | +| `api` | When pipeline has been triggered by a second pipelines API (not triggers API). | +| `external` | When using CI services other than GitLab. | +| `pipelines` | For multi-project triggers, created using the API with `CI_JOB_TOKEN`. | +| `pushes` | Pipeline is triggered by a `git push` by the user. | +| `schedules` | For [scheduled pipelines][schedules]. | +| `triggers` | For pipelines created using a trigger token. | +| `web` | For pipelines created using **Run pipeline** button in GitLab UI (under your project's **Pipelines**). | +| `merge_requests` | When a merge request is created or updated (See [pipelines for merge requests](../merge_request_pipelines/index.md)). | In the example below, `job` will run only for refs that start with `issue-`, whereas all branches will be skipped: @@ -391,6 +392,24 @@ job: The above example will run `job` for all branches on `gitlab-org/gitlab-ce`, except master. +If a job does not have neither `only` nor `except` rule, +`only: ['branches', 'tags']` is set by default. + +For example, + +```yaml +job: + script: echo 'test' +``` + +is translated to + +```yaml +job: + script: echo 'test' + only: ['branches', 'tags'] +``` + ## `only` and `except` (complex) > `refs` and `kubernetes` policies introduced in GitLab 10.0 diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index b7990e1b558..55aed023325 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -368,6 +368,16 @@ You can combine one or more of the following: = link_to 'Help page', help_page_path('user/permissions') ``` +### GitLab `/help` tests + +Several [rspec tests](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/features/help_pages_spec.rb) +are run to ensure GitLab documentation renders and works correctly. In particular, that [main docs landing page](../../README.md) will work correctly from `/help`. +For example, [GitLab.com's `/help`](https://gitlab.com/help). + +CAUTION: **Caution:** +Because the rspec tests only run in a full pipeline, and not a special [docs-only pipeline](#branch-naming), it is possible +to merge changes that will break `master` from a merge request with a successful docs-only pipeline run. + ## General Documentation vs Technical Articles ### General documentation @@ -552,6 +562,7 @@ Currently, the following tests are in place: As CE is merged into EE once a day, it's important to avoid merge conflicts. Submitting an EE-equivalent merge request cherry-picking all commits from CE to EE is essential to avoid them. +1. In a full pipeline, tests for [`/help`](#gitlab-help-tests). ### Linting diff --git a/doc/development/feature_flags.md b/doc/development/feature_flags.md index 1019a1fd0e2..b6161cd6163 100644 --- a/doc/development/feature_flags.md +++ b/doc/development/feature_flags.md @@ -113,7 +113,15 @@ feature flag. You can stub a feature flag as follows: stub_feature_flags(my_feature_flag: false) ``` -## Enabling a feature flag +## Enabling a feature flag (in development) + +In the rails console (`rails c`), enter the following command to enable your feature flag + +```ruby +Feature.enable(:feature_flag_name) +``` + +## Enabling a feature flag (in production) Check how to [roll out changes using feature flags](rolling_out_changes_using_feature_flags.md). diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index 5b54b6ecdd5..85d8d804133 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -259,6 +259,16 @@ all your changes will be available to preview by anyone with the Review Apps lin [Read more about Review Apps.](../../../ci/review_apps/index.md) +## Pipelines for merge requests + +When a developer updates a merge request, a pipeline should quickly report back +its result to the developer, but often pipelines take long time to complete +because general branch pipelines contain unnecessary jobs from the merge request standpoint. +You can customize a specific pipeline structure for merge requests in order to +speed the cycle up by running only important jobs. + +Learn more about [pipelines for merge requests](../../../ci/merge_request_pipelines/index.md). + ## Pipeline status in merge requests If you've set up [GitLab CI/CD](../../../ci/README.md) in your project, diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index fde8561c16c..d10d4f2f746 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -143,6 +143,7 @@ excluded_attributes: statuses: - :trace - :token + - :token_encrypted - :when - :artifacts_file - :artifacts_metadata diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index f6c85102830..4b0dc4c9b69 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -226,9 +226,10 @@ describe GroupsController do end context 'searching' do - # Remove as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/52271 before do + # Remove in https://gitlab.com/gitlab-org/gitlab-ce/issues/54643 stub_feature_flags(use_cte_for_group_issues_search: false) + stub_feature_flags(use_subquery_for_group_issues_search: true) end it 'works with popularity sort' do diff --git a/spec/controllers/projects/settings/repository_controller_spec.rb b/spec/controllers/projects/settings/repository_controller_spec.rb index 69ec971bb75..70f79a47e63 100644 --- a/spec/controllers/projects/settings/repository_controller_spec.rb +++ b/spec/controllers/projects/settings/repository_controller_spec.rb @@ -19,12 +19,14 @@ describe Projects::Settings::RepositoryController do end describe 'PUT cleanup' do + before do + allow(RepositoryCleanupWorker).to receive(:perform_async) + end + def do_put! object_map = fixture_file_upload('spec/fixtures/bfg_object_map.txt') - Sidekiq::Testing.fake! do - put :cleanup, namespace_id: project.namespace, project_id: project, project: { object_map: object_map } - end + put :cleanup, namespace_id: project.namespace, project_id: project, project: { object_map: object_map } end context 'feature enabled' do @@ -34,7 +36,7 @@ describe Projects::Settings::RepositoryController do do_put! expect(response).to redirect_to project_settings_repository_path(project) - expect(RepositoryCleanupWorker.jobs.count).to eq(1) + expect(RepositoryCleanupWorker).to have_received(:perform_async).once end end diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index 996040fde02..055a0c83a11 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/spec/features/projects/labels/update_prioritization_spec.rb @@ -115,6 +115,21 @@ describe 'Prioritize labels' do end end + it 'user can see a primary button when there are only prioritized labels', :js do + visit project_labels_path(project) + + page.within('.other-labels') do + all('.js-toggle-priority').each do |el| + el.click + end + wait_for_requests + end + + page.within('.breadcrumbs-container') do + expect(page).to have_link('New label') + end + end + it 'shows a help message about prioritized labels' do visit project_labels_path(project) diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 515f6f70b99..80f7232f282 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -640,4 +640,131 @@ describe IssuesFinder do end end end + + describe '#use_subquery_for_search?' do + let(:finder) { described_class.new(nil, params) } + + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + stub_feature_flags(use_subquery_for_group_issues_search: true) + end + + context 'when there is no search param' do + let(:params) { { attempt_group_search_optimizations: true } } + + it 'returns false' do + expect(finder.use_subquery_for_search?).to be_falsey + end + end + + context 'when the database is not Postgres' do + let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } + + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + end + + it 'returns false' do + expect(finder.use_subquery_for_search?).to be_falsey + end + end + + context 'when the attempt_group_search_optimizations param is falsey' do + let(:params) { { search: 'foo' } } + + it 'returns false' do + expect(finder.use_subquery_for_search?).to be_falsey + end + end + + context 'when the use_subquery_for_group_issues_search flag is disabled' do + let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } + + before do + stub_feature_flags(use_subquery_for_group_issues_search: false) + end + + it 'returns false' do + expect(finder.use_subquery_for_search?).to be_falsey + end + end + + context 'when all conditions are met' do + let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } + + it 'returns true' do + expect(finder.use_subquery_for_search?).to be_truthy + end + end + end + + describe '#use_cte_for_search?' do + let(:finder) { described_class.new(nil, params) } + + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + stub_feature_flags(use_cte_for_group_issues_search: true) + stub_feature_flags(use_subquery_for_group_issues_search: false) + end + + context 'when there is no search param' do + let(:params) { { attempt_group_search_optimizations: true } } + + it 'returns false' do + expect(finder.use_cte_for_search?).to be_falsey + end + end + + context 'when the database is not Postgres' do + let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } + + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + end + + it 'returns false' do + expect(finder.use_cte_for_search?).to be_falsey + end + end + + context 'when the attempt_group_search_optimizations param is falsey' do + let(:params) { { search: 'foo' } } + + it 'returns false' do + expect(finder.use_cte_for_search?).to be_falsey + end + end + + context 'when the use_cte_for_group_issues_search flag is disabled' do + let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } + + before do + stub_feature_flags(use_cte_for_group_issues_search: false) + end + + it 'returns false' do + expect(finder.use_cte_for_search?).to be_falsey + end + end + + context 'when use_subquery_for_search? is true' do + let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } + + before do + stub_feature_flags(use_subquery_for_group_issues_search: true) + end + + it 'returns false' do + expect(finder.use_cte_for_search?).to be_falsey + end + end + + context 'when all conditions are met' do + let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } + + it 'returns true' do + expect(finder.use_cte_for_search?).to be_truthy + end + end + end end diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index 4b339a0553f..55ce19927e0 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -29,6 +29,7 @@ import actions, { } from '~/diffs/store/actions'; import * as types from '~/diffs/store/mutation_types'; import axios from '~/lib/utils/axios_utils'; +import mockDiffFile from 'spec/diffs/mock_data/diff_file'; import testAction from '../../helpers/vuex_action_helper'; describe('DiffsStoreActions', () => { @@ -607,11 +608,18 @@ describe('DiffsStoreActions', () => { }); describe('saveDiffDiscussion', () => { - beforeEach(() => { - spyOnDependency(actions, 'getNoteFormData').and.returnValue('testData'); - }); - it('dispatches actions', done => { + const commitId = 'something'; + const formData = { + diffFile: { ...mockDiffFile }, + noteableData: {}, + }; + const note = {}; + const state = { + commit: { + id: commitId, + }, + }; const dispatch = jasmine.createSpy('dispatch').and.callFake(name => { switch (name) { case 'saveNote': @@ -625,11 +633,19 @@ describe('DiffsStoreActions', () => { } }); - saveDiffDiscussion({ dispatch }, { note: {}, formData: {} }) + saveDiffDiscussion({ state, dispatch }, { note, formData }) .then(() => { - expect(dispatch.calls.argsFor(0)).toEqual(['saveNote', 'testData', { root: true }]); - expect(dispatch.calls.argsFor(1)).toEqual(['updateDiscussion', 'test', { root: true }]); - expect(dispatch.calls.argsFor(2)).toEqual(['assignDiscussionsToDiff', ['discussion']]); + const { calls } = dispatch; + + expect(calls.count()).toBe(5); + expect(calls.argsFor(0)).toEqual(['saveNote', jasmine.any(Object), { root: true }]); + + const postData = calls.argsFor(0)[1]; + + expect(postData.data.note.commit_id).toBe(commitId); + + expect(calls.argsFor(1)).toEqual(['updateDiscussion', 'test', { root: true }]); + expect(calls.argsFor(2)).toEqual(['assignDiscussionsToDiff', ['discussion']]); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index 717f983da65..f096638e3d6 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -150,7 +150,7 @@ describe('DiffsStoreUtils', () => { note: { noteable_type: options.noteableType, noteable_id: options.noteableData.id, - commit_id: '', + commit_id: undefined, type: DIFF_NOTE_TYPE, line_code: options.noteTargetLine.line_code, note: options.note, @@ -209,7 +209,7 @@ describe('DiffsStoreUtils', () => { note: { noteable_type: options.noteableType, noteable_id: options.noteableData.id, - commit_id: '', + commit_id: undefined, type: LEGACY_DIFF_NOTE_TYPE, line_code: options.noteTargetLine.line_code, note: options.note, diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 7df129da95a..bae5b21c26f 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -287,6 +287,7 @@ project: - statistics - container_repositories - uploads +- file_uploads - import_state - members_and_requesters - build_trace_section_names diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 77b07cf1ac9..35415030154 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -20,7 +20,7 @@ describe Appearance do end context 'with uploads' do - it_behaves_like 'model with mounted uploader', false do + it_behaves_like 'model with uploads', false do let(:model_object) { create(:appearance, :with_logo) } let(:upload_attribute) { :logo } let(:uploader_class) { AttachmentUploader } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 4cdcae5f670..89f78f629d4 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1925,7 +1925,7 @@ describe Ci::Build do context 'when token is empty' do before do - build.token = nil + build.update_columns(token: nil, token_encrypted: nil) end it { is_expected.to be_nil} @@ -2141,7 +2141,7 @@ describe Ci::Build do end before do - build.token = 'my-token' + build.set_token('my-token') build.yaml_variables = [] end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 97e50809647..47daa79873e 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -18,7 +18,7 @@ describe Clusters::Applications::Runner do let(:application) { create(:clusters_applications_runner, :scheduled, version: '0.1.30') } it 'updates the application version' do - expect(application.reload.version).to eq('0.1.38') + expect(application.reload.version).to eq('0.1.39') end end end @@ -46,7 +46,7 @@ describe Clusters::Applications::Runner do it 'should be initialized with 4 arguments' do expect(subject.name).to eq('runner') expect(subject.chart).to eq('runner/gitlab-runner') - expect(subject.version).to eq('0.1.38') + expect(subject.version).to eq('0.1.39') expect(subject).not_to be_rbac expect(subject.repository).to eq('https://charts.gitlab.io') expect(subject.files).to eq(gitlab_runner.files) @@ -64,7 +64,7 @@ describe Clusters::Applications::Runner do let(:gitlab_runner) { create(:clusters_applications_runner, :errored, runner: ci_runner, version: '0.1.13') } it 'should be initialized with the locked version' do - expect(subject.version).to eq('0.1.38') + expect(subject.version).to eq('0.1.39') end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 2a0039a0635..a2d2d77746d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -204,7 +204,7 @@ describe Commit do message = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id felis id blandit. Vivamus egestas lacinia lacus, sed rutrum mauris.' allow(commit).to receive(:safe_message).and_return(message) - expect(commit.title).to eq('Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id felis…') + expect(commit.title).to eq('Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id...') end it "truncates a message with a newline before 80 characters at the newline" do diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 0cdf430e9ab..55d83bc3a6b 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -351,3 +351,89 @@ describe PersonalAccessToken, 'TokenAuthenticatable' do end end end + +describe Ci::Build, 'TokenAuthenticatable' do + let(:token_field) { :token } + let(:build) { FactoryBot.build(:ci_build) } + + it_behaves_like 'TokenAuthenticatable' + + describe 'generating new token' do + context 'token is not generated yet' do + describe 'token field accessor' do + it 'makes it possible to access token' do + expect(build.token).to be_nil + + build.save! + + expect(build.token).to be_present + end + end + + describe "ensure_token" do + subject { build.ensure_token } + + it { is_expected.to be_a String } + it { is_expected.not_to be_blank } + + it 'does not persist token' do + expect(build).not_to be_persisted + end + end + + describe 'ensure_token!' do + it 'persists a new token' do + expect(build.ensure_token!).to eq build.reload.token + expect(build).to be_persisted + end + + it 'persists new token as an encrypted string' do + build.ensure_token! + + encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(build.token) + + expect(build.read_attribute('token_encrypted')).to eq encrypted + end + + it 'does not persist a token in a clear text' do + build.ensure_token! + + expect(build.read_attribute('token')).to be_nil + end + end + end + + describe '#reset_token!' do + it 'persists a new token' do + build.save! + + build.token.yield_self do |previous_token| + build.reset_token! + + expect(build.token).not_to eq previous_token + expect(build.token).to be_a String + end + end + end + end + + describe 'setting a new token' do + subject { build.set_token('0123456789') } + + it 'returns the token' do + expect(subject).to eq '0123456789' + end + + it 'writes a new encrypted token' do + expect(build.read_attribute('token_encrypted')).to be_nil + expect(subject).to eq '0123456789' + expect(build.read_attribute('token_encrypted')).to be_present + end + + it 'does not write a new cleartext token' do + expect(build.read_attribute('token')).to be_nil + expect(subject).to eq '0123456789' + expect(build.read_attribute('token')).to be_nil + end + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 87aa5a46c21..e63881242f6 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -739,7 +739,7 @@ describe Group do end context 'with uploads' do - it_behaves_like 'model with mounted uploader', true do + it_behaves_like 'model with uploads', true do let(:model_object) { create(:group, :with_avatar) } let(:upload_attribute) { :avatar } let(:uploader_class) { AttachmentUploader } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 50920d9d1fc..93c83fd21fd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3898,7 +3898,7 @@ describe Project do end context 'with uploads' do - it_behaves_like 'model with mounted uploader', true do + it_behaves_like 'model with uploads', true do let(:model_object) { create(:project, :with_avatar) } let(:upload_attribute) { :avatar } let(:uploader_class) { AttachmentUploader } diff --git a/spec/models/uploads/fog_spec.rb b/spec/models/uploads/fog_spec.rb new file mode 100644 index 00000000000..4a44cf5ab0f --- /dev/null +++ b/spec/models/uploads/fog_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Uploads::Fog do + let(:data_store) { described_class.new } + + before do + stub_uploads_object_storage(FileUploader) + end + + describe '#available?' do + subject { data_store.available? } + + context 'when object storage is enabled' do + it { is_expected.to be_truthy } + end + + context 'when object storage is disabled' do + before do + stub_uploads_object_storage(FileUploader, enabled: false) + end + + it { is_expected.to be_falsy } + end + end + + context 'model with uploads' do + let(:project) { create(:project) } + let(:relation) { project.uploads } + + describe '#keys' do + let!(:uploads) { create_list(:upload, 2, :object_storage, uploader: FileUploader, model: project) } + subject { data_store.keys(relation) } + + it 'returns keys' do + is_expected.to match_array(relation.pluck(:path)) + end + end + + describe '#delete_keys' do + let(:keys) { data_store.keys(relation) } + let!(:uploads) { create_list(:upload, 2, :with_file, :issuable_upload, model: project) } + subject { data_store.delete_keys(keys) } + + before do + uploads.each { |upload| upload.build_uploader.migrate!(2) } + end + + it 'deletes multiple data' do + paths = relation.pluck(:path) + + ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection| + paths.each do |path| + expect(connection.get_object('uploads', path)[:body]).not_to be_nil + end + end + + subject + + ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection| + paths.each do |path| + expect { connection.get_object('uploads', path)[:body] }.to raise_error(Excon::Error::NotFound) + end + end + end + end + end +end diff --git a/spec/models/uploads/local_spec.rb b/spec/models/uploads/local_spec.rb new file mode 100644 index 00000000000..3468399f370 --- /dev/null +++ b/spec/models/uploads/local_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Uploads::Local do + let(:data_store) { described_class.new } + + before do + stub_uploads_object_storage(FileUploader) + end + + context 'model with uploads' do + let(:project) { create(:project) } + let(:relation) { project.uploads } + + describe '#keys' do + let!(:uploads) { create_list(:upload, 2, uploader: FileUploader, model: project) } + subject { data_store.keys(relation) } + + it 'returns keys' do + is_expected.to match_array(relation.map(&:absolute_path)) + end + end + + describe '#delete_keys' do + let(:keys) { data_store.keys(relation) } + let!(:uploads) { create_list(:upload, 2, :with_file, :issuable_upload, model: project) } + subject { data_store.delete_keys(keys) } + + it 'deletes multiple data' do + paths = relation.map(&:absolute_path) + + paths.each do |path| + expect(File.exist?(path)).to be_truthy + end + + subject + + paths.each do |path| + expect(File.exist?(path)).to be_falsey + end + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6cb27246f06..ff075e65c76 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3231,7 +3231,7 @@ describe User do end context 'with uploads' do - it_behaves_like 'model with mounted uploader', false do + it_behaves_like 'model with uploads', false do let(:model_object) { create(:user, :with_avatar) } let(:upload_attribute) { :avatar } let(:uploader_class) { AttachmentUploader } diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index e779675744c..87185891470 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -20,9 +20,9 @@ describe Ci::RetryBuildService do CLONE_ACCESSORS = described_class::CLONE_ACCESSORS REJECT_ACCESSORS = - %i[id status user token coverage trace runner artifacts_expire_at - artifacts_file artifacts_metadata artifacts_size created_at - updated_at started_at finished_at queued_at erased_by + %i[id status user token token_encrypted coverage trace runner + artifacts_expire_at artifacts_file artifacts_metadata artifacts_size + created_at updated_at started_at finished_at queued_at erased_by erased_at auto_canceled_by job_artifacts job_artifacts_archive job_artifacts_metadata job_artifacts_trace job_artifacts_junit job_artifacts_sast job_artifacts_dependency_scanning diff --git a/spec/support/shared_examples/models/with_uploads_shared_examples.rb b/spec/support/shared_examples/models/with_uploads_shared_examples.rb index 47ad0c6345d..1d11b855459 100644 --- a/spec/support/shared_examples/models/with_uploads_shared_examples.rb +++ b/spec/support/shared_examples/models/with_uploads_shared_examples.rb @@ -1,6 +1,6 @@ require 'spec_helper' -shared_examples_for 'model with mounted uploader' do |supports_fileuploads| +shared_examples_for 'model with uploads' do |supports_fileuploads| describe '.destroy' do before do stub_uploads_object_storage(uploader_class) @@ -8,16 +8,62 @@ shared_examples_for 'model with mounted uploader' do |supports_fileuploads| model_object.public_send(upload_attribute).migrate!(ObjectStorage::Store::REMOTE) end - it 'deletes remote uploads' do - expect_any_instance_of(CarrierWave::Storage::Fog::File).to receive(:delete).and_call_original + context 'with mounted uploader' do + it 'deletes remote uploads' do + expect_any_instance_of(CarrierWave::Storage::Fog::File).to receive(:delete).and_call_original - expect { model_object.destroy }.to change { Upload.count }.by(-1) + expect { model_object.destroy }.to change { Upload.count }.by(-1) + end end - it 'deletes any FileUploader uploads which are not mounted', skip: !supports_fileuploads do - create(:upload, uploader: FileUploader, model: model_object) + context 'with not mounted uploads', :sidekiq, skip: !supports_fileuploads do + context 'with local files' do + let!(:uploads) { create_list(:upload, 2, uploader: FileUploader, model: model_object) } - expect { model_object.destroy }.to change { Upload.count }.by(-2) + it 'deletes any FileUploader uploads which are not mounted' do + expect { model_object.destroy }.to change { Upload.count }.by(-3) + end + + it 'deletes local files' do + expect_any_instance_of(Uploads::Local).to receive(:delete_keys).with(uploads.map(&:absolute_path)) + + model_object.destroy + end + end + + context 'with remote files' do + let!(:uploads) { create_list(:upload, 2, :object_storage, uploader: FileUploader, model: model_object) } + + it 'deletes any FileUploader uploads which are not mounted' do + expect { model_object.destroy }.to change { Upload.count }.by(-3) + end + + it 'deletes remote files' do + expect_any_instance_of(Uploads::Fog).to receive(:delete_keys).with(uploads.map(&:path)) + + model_object.destroy + end + end + + describe 'destroy strategy depending on feature flag' do + let!(:upload) { create(:upload, uploader: FileUploader, model: model_object) } + + it 'does not destroy uploads by default' do + expect(model_object).to receive(:delete_uploads) + expect(model_object).not_to receive(:destroy_uploads) + + model_object.destroy + end + + it 'uses before destroy callback if feature flag is disabled' do + stub_feature_flags(fast_destroy_uploads: false) + + expect(model_object).to receive(:destroy_uploads) + expect(model_object).not_to receive(:delete_uploads) + + model_object.destroy + end + end end end end |
