diff options
| author | Alessio Caiazza <acaiazza@gitlab.com> | 2018-06-04 13:57:51 +0200 |
|---|---|---|
| committer | Alessio Caiazza <acaiazza@gitlab.com> | 2018-06-04 13:57:51 +0200 |
| commit | 8ffcaf3b85fdf716ffeacf4e9d4c89b922de50b9 (patch) | |
| tree | 2e6acfa369e3e659ef4c6afb9290ad122a8fea5c | |
| parent | 1841da16abe864b3dae19636fee9e9bbe9a01b56 (diff) | |
| parent | 23ae072ba2d601a3639a32f5a29302c430106bb8 (diff) | |
| download | gitlab-ce-11-0-stable-prepare-rc2.tar.gz | |
Merge branch 'master' into 11-0-stable-prepare-rc211-0-stable-prepare-rc2
207 files changed, 1805 insertions, 874 deletions
diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index a8a18875682..1996c504476 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -7.1.2 +7.1.3 diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index fae6e3d04b2..80895903a15 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -4.2.1 +4.3.0 @@ -342,7 +342,7 @@ group :development, :test do gem 'capybara', '~> 2.15' gem 'capybara-screenshot', '~> 1.0.0' - gem 'selenium-webdriver', '~> 3.5' + gem 'selenium-webdriver', '~> 3.12' gem 'spring', '~> 2.0.0' gem 'spring-commands-rspec', '~> 1.0.4' @@ -384,7 +384,7 @@ group :test do gem 'test-prof', '~> 0.2.5' end -gem 'octokit', '~> 4.8' +gem 'octokit', '~> 4.9' gem 'mail_room', '~> 0.9.1' diff --git a/Gemfile.lock b/Gemfile.lock index 4d2bd62bec0..e6e8f3d11bc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -115,7 +115,7 @@ GEM mime-types (>= 1.16) cause (0.1) charlock_holmes (0.7.6) - childprocess (0.7.0) + childprocess (0.9.0) ffi (~> 1.0, >= 1.0.11) chronic (0.10.2) chronic_duration (0.10.6) @@ -517,7 +517,7 @@ GEM multi_json (~> 1.3) multi_xml (~> 0.5) rack (>= 1.2, < 3) - octokit (4.8.0) + octokit (4.9.0) sawyer (~> 0.8.0, >= 0.5.3) omniauth (1.8.1) hashie (>= 3.4.6, < 3.6.0) @@ -540,7 +540,7 @@ GEM omniauth-github (1.3.0) omniauth (~> 1.5) omniauth-oauth2 (>= 1.4.0, < 2.0) - omniauth-gitlab (1.0.2) + omniauth-gitlab (1.0.3) omniauth (~> 1.0) omniauth-oauth2 (~> 1.0) omniauth-google-oauth2 (0.5.3) @@ -828,9 +828,9 @@ GEM activesupport (>= 3.1) select2-rails (3.5.9.3) thor (~> 0.14) - selenium-webdriver (3.5.0) + selenium-webdriver (3.12.0) childprocess (~> 0.5) - rubyzip (~> 1.0) + rubyzip (~> 1.2) sentry-raven (2.7.2) faraday (>= 0.7.6, < 1.0) settingslogic (2.0.9) @@ -1084,7 +1084,7 @@ DEPENDENCIES net-ssh (~> 4.2.0) nokogiri (~> 1.8.2) oauth2 (~> 1.4) - octokit (~> 4.8) + octokit (~> 4.9) omniauth (~> 1.8) omniauth-auth0 (~> 2.0.0) omniauth-authentiq (~> 0.3.3) @@ -1154,7 +1154,7 @@ DEPENDENCIES scss_lint (~> 0.56.0) seed-fu (~> 2.3.7) select2-rails (~> 3.5.9) - selenium-webdriver (~> 3.5) + selenium-webdriver (~> 3.12) sentry-raven (~> 2.7) settingslogic (~> 2.0.9) sham_rack (~> 1.3.6) diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index 47bf55b1c23..000938e475f 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -11,6 +11,7 @@ const Api = { projectPath: '/api/:version/projects/:id', projectLabelsPath: '/:namespace_path/:project_path/labels', mergeRequestPath: '/api/:version/projects/:id/merge_requests/:mrid', + mergeRequestsPath: '/api/:version/merge_requests', mergeRequestChangesPath: '/api/:version/projects/:id/merge_requests/:mrid/changes', mergeRequestVersionsPath: '/api/:version/projects/:id/merge_requests/:mrid/versions', groupLabelsPath: '/groups/:namespace_path/-/labels', @@ -107,6 +108,12 @@ const Api = { return axios.get(url); }, + mergeRequests(params = {}) { + const url = Api.buildUrl(Api.mergeRequestsPath); + + return axios.get(url, { params }); + }, + mergeRequestChanges(projectPath, mergeRequestId) { const url = Api.buildUrl(Api.mergeRequestChangesPath) .replace(':id', encodeURIComponent(projectPath)) diff --git a/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue b/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue index f14fcdc88ed..0ac0af2feaa 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue @@ -54,7 +54,7 @@ export default { placement: 'top', content: sprintf( __(` - The character highligher helps you keep the subject line to %{titleLength} characters + The character highlighter helps you keep the subject line to %{titleLength} characters and wrap the body at %{bodyLength} so they are readable in git. `), { titleLength: MAX_TITLE_LENGTH, bodyLength: MAX_BODY_LENGTH }, diff --git a/app/assets/javascripts/ide/stores/index.js b/app/assets/javascripts/ide/stores/index.js index 5d26cb8d626..f8ce8a67ec0 100644 --- a/app/assets/javascripts/ide/stores/index.js +++ b/app/assets/javascripts/ide/stores/index.js @@ -6,6 +6,7 @@ import * as getters from './getters'; import mutations from './mutations'; import commitModule from './modules/commit'; import pipelines from './modules/pipelines'; +import mergeRequests from './modules/merge_requests'; Vue.use(Vuex); @@ -18,6 +19,7 @@ export const createStore = () => modules: { commit: commitModule, pipelines, + mergeRequests, }, }); diff --git a/app/assets/javascripts/ide/stores/modules/merge_requests/actions.js b/app/assets/javascripts/ide/stores/modules/merge_requests/actions.js new file mode 100644 index 00000000000..d3050183bd3 --- /dev/null +++ b/app/assets/javascripts/ide/stores/modules/merge_requests/actions.js @@ -0,0 +1,25 @@ +import { __ } from '../../../../locale'; +import Api from '../../../../api'; +import flash from '../../../../flash'; +import * as types from './mutation_types'; + +export const requestMergeRequests = ({ commit }) => commit(types.REQUEST_MERGE_REQUESTS); +export const receiveMergeRequestsError = ({ commit }) => { + flash(__('Error loading merge requests.')); + commit(types.RECEIVE_MERGE_REQUESTS_ERROR); +}; +export const receiveMergeRequestsSuccess = ({ commit }, data) => + commit(types.RECEIVE_MERGE_REQUESTS_SUCCESS, data); + +export const fetchMergeRequests = ({ dispatch, state: { scope, state } }, search = '') => { + dispatch('requestMergeRequests'); + dispatch('resetMergeRequests'); + + Api.mergeRequests({ scope, state, search }) + .then(({ data }) => dispatch('receiveMergeRequestsSuccess', data)) + .catch(() => dispatch('receiveMergeRequestsError')); +}; + +export const resetMergeRequests = ({ commit }) => commit(types.RESET_MERGE_REQUESTS); + +export default () => {}; diff --git a/app/assets/javascripts/ide/stores/modules/merge_requests/constants.js b/app/assets/javascripts/ide/stores/modules/merge_requests/constants.js new file mode 100644 index 00000000000..64b7763f257 --- /dev/null +++ b/app/assets/javascripts/ide/stores/modules/merge_requests/constants.js @@ -0,0 +1,10 @@ +export const scopes = { + assignedToMe: 'assigned-to-me', + createdByMe: 'created-by-me', +}; + +export const states = { + opened: 'opened', + closed: 'closed', + merged: 'merged', +}; diff --git a/app/assets/javascripts/ide/stores/modules/merge_requests/index.js b/app/assets/javascripts/ide/stores/modules/merge_requests/index.js new file mode 100644 index 00000000000..04e7e0f08f1 --- /dev/null +++ b/app/assets/javascripts/ide/stores/modules/merge_requests/index.js @@ -0,0 +1,10 @@ +import state from './state'; +import * as actions from './actions'; +import mutations from './mutations'; + +export default { + namespaced: true, + state: state(), + actions, + mutations, +}; diff --git a/app/assets/javascripts/ide/stores/modules/merge_requests/mutation_types.js b/app/assets/javascripts/ide/stores/modules/merge_requests/mutation_types.js new file mode 100644 index 00000000000..0badddcbae7 --- /dev/null +++ b/app/assets/javascripts/ide/stores/modules/merge_requests/mutation_types.js @@ -0,0 +1,5 @@ +export const REQUEST_MERGE_REQUESTS = 'REQUEST_MERGE_REQUESTS'; +export const RECEIVE_MERGE_REQUESTS_ERROR = 'RECEIVE_MERGE_REQUESTS_ERROR'; +export const RECEIVE_MERGE_REQUESTS_SUCCESS = 'RECEIVE_MERGE_REQUESTS_SUCCESS'; + +export const RESET_MERGE_REQUESTS = 'RESET_MERGE_REQUESTS'; diff --git a/app/assets/javascripts/ide/stores/modules/merge_requests/mutations.js b/app/assets/javascripts/ide/stores/modules/merge_requests/mutations.js new file mode 100644 index 00000000000..98102a68e08 --- /dev/null +++ b/app/assets/javascripts/ide/stores/modules/merge_requests/mutations.js @@ -0,0 +1,26 @@ +/* eslint-disable no-param-reassign */ +import * as types from './mutation_types'; + +export default { + [types.REQUEST_MERGE_REQUESTS](state) { + state.isLoading = true; + }, + [types.RECEIVE_MERGE_REQUESTS_ERROR](state) { + state.isLoading = false; + }, + [types.RECEIVE_MERGE_REQUESTS_SUCCESS](state, data) { + state.isLoading = false; + state.mergeRequests = data.map(mergeRequest => ({ + id: mergeRequest.id, + iid: mergeRequest.iid, + title: mergeRequest.title, + projectId: mergeRequest.project_id, + projectPathWithNamespace: mergeRequest.web_url + .replace(`${gon.gitlab_url}/`, '') + .replace(`/merge_requests/${mergeRequest.iid}`, ''), + })); + }, + [types.RESET_MERGE_REQUESTS](state) { + state.mergeRequests = []; + }, +}; diff --git a/app/assets/javascripts/ide/stores/modules/merge_requests/state.js b/app/assets/javascripts/ide/stores/modules/merge_requests/state.js new file mode 100644 index 00000000000..2947b686c1c --- /dev/null +++ b/app/assets/javascripts/ide/stores/modules/merge_requests/state.js @@ -0,0 +1,8 @@ +import { scopes, states } from './constants'; + +export default () => ({ + isLoading: false, + mergeRequests: [], + scope: scopes.assignedToMe, + state: states.opened, +}); diff --git a/app/assets/javascripts/integrations/integration_settings_form.js b/app/assets/javascripts/integrations/integration_settings_form.js index 741894b5e6c..cdb75752b4e 100644 --- a/app/assets/javascripts/integrations/integration_settings_form.js +++ b/app/assets/javascripts/integrations/integration_settings_form.js @@ -101,13 +101,19 @@ export default class IntegrationSettingsForm { return axios.put(this.testEndPoint, formData) .then(({ data }) => { if (data.error) { - flash(`${data.message} ${data.service_response}`, 'alert', document, { - title: 'Save anyway', - clickHandler: (e) => { - e.preventDefault(); - this.$form.submit(); - }, - }); + let flashActions; + + if (data.test_failed) { + flashActions = { + title: 'Save anyway', + clickHandler: (e) => { + e.preventDefault(); + this.$form.submit(); + }, + }; + } + + flash(`${data.message} ${data.service_response}`, 'alert', document, flashActions); } else { this.$form.submit(); } diff --git a/app/assets/stylesheets/bootstrap_migration.scss b/app/assets/stylesheets/bootstrap_migration.scss index 4a4da027c92..e24f8b1d4e8 100644 --- a/app/assets/stylesheets/bootstrap_migration.scss +++ b/app/assets/stylesheets/bootstrap_migration.scss @@ -36,6 +36,17 @@ html [type="button"], cursor: pointer; } +input[type="file"] { + // Bootstrap 4 file input height is taller by default + // which makes them look ugly + line-height: 1; +} + +b, +strong { + font-weight: bold; +} + a { color: $gl-link-color; } @@ -48,6 +59,12 @@ a { } } +code { + padding: 2px 4px; + background-color: $red-100; + border-radius: 3px; +} + table { // Remove any table border lines border-spacing: 0; diff --git a/app/assets/stylesheets/framework/modal.scss b/app/assets/stylesheets/framework/modal.scss index ed5a1c91d8f..a7896cc3fc3 100644 --- a/app/assets/stylesheets/framework/modal.scss +++ b/app/assets/stylesheets/framework/modal.scss @@ -1,6 +1,5 @@ .modal-header { background-color: $modal-body-bg; - padding: #{3 * $grid-size} #{2 * $grid-size}; .page-title, .modal-title { diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index ed0bfbbe08b..97b821e0cb9 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -340,10 +340,6 @@ code { } } -a > code { - color: $link-color; -} - .monospace { font-family: $monospace_font; } diff --git a/app/controllers/admin/runner_projects_controller.rb b/app/controllers/admin/runner_projects_controller.rb index 7ed2de71028..7aba77d8129 100644 --- a/app/controllers/admin/runner_projects_controller.rb +++ b/app/controllers/admin/runner_projects_controller.rb @@ -4,9 +4,7 @@ class Admin::RunnerProjectsController < Admin::ApplicationController def create @runner = Ci::Runner.find(params[:runner_project][:runner_id]) - runner_project = @runner.assign_to(@project, current_user) - - if runner_project.persisted? + if @runner.assign_to(@project, current_user) redirect_to admin_runner_path(@runner) else redirect_to admin_runner_path(@runner), alert: 'Failed adding runner to project' diff --git a/app/controllers/projects/runner_projects_controller.rb b/app/controllers/projects/runner_projects_controller.rb index 0ec2490655f..a080724634b 100644 --- a/app/controllers/projects/runner_projects_controller.rb +++ b/app/controllers/projects/runner_projects_controller.rb @@ -9,9 +9,8 @@ class Projects::RunnerProjectsController < Projects::ApplicationController return head(403) unless can?(current_user, :assign_runner, @runner) path = project_runners_path(project) - runner_project = @runner.assign_to(project, current_user) - if runner_project.persisted? + if @runner.assign_to(project, current_user) redirect_to path else redirect_to path, alert: 'Failed adding runner to project' diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index a5ea9ff7ed7..690596b12db 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -41,13 +41,13 @@ class Projects::ServicesController < Projects::ApplicationController if outcome[:success] {} else - { error: true, message: 'Test failed.', service_response: outcome[:result].to_s } + { error: true, message: 'Test failed.', service_response: outcome[:result].to_s, test_failed: true } end else - { error: true, message: 'Validations failed.', service_response: @service.errors.full_messages.join(',') } + { error: true, message: 'Validations failed.', service_response: @service.errors.full_messages.join(','), test_failed: false } end rescue Gitlab::HTTP::BlockedUrlError => e - { error: true, message: 'Test failed.', service_response: e.message } + { error: true, message: 'Test failed.', service_response: e.message, test_failed: true } end def success_message diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 761c1252fc8..f7dafca7834 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -25,14 +25,22 @@ module SearchHelper return unless collection.count > 0 from = collection.offset_value + 1 - to = collection.offset_value + collection.length + to = collection.offset_value + collection.count count = collection.total_count "Showing #{from} - #{to} of #{count} #{scope.humanize(capitalize: false)} for \"#{term}\"" end + def find_project_for_result_blob(result) + @project + end + def parse_search_result(result) - Gitlab::ProjectSearchResults.parse_search_result(result) + result + end + + def search_blob_title(project, filename) + filename end private diff --git a/app/helpers/webpack_helper.rb b/app/helpers/webpack_helper.rb index e12e4ba70e9..72f6b397046 100644 --- a/app/helpers/webpack_helper.rb +++ b/app/helpers/webpack_helper.rb @@ -1,5 +1,3 @@ -require 'gitlab/webpack/manifest' - module WebpackHelper def webpack_bundle_tag(bundle) javascript_include_tag(*webpack_entrypoint_paths(bundle)) diff --git a/app/models/badge.rb b/app/models/badge.rb index f7e10c2ebfc..265c5d872d4 100644 --- a/app/models/badge.rb +++ b/app/models/badge.rb @@ -18,7 +18,7 @@ class Badge < ActiveRecord::Base scope :order_created_at_asc, -> { reorder(created_at: :asc) } - validates :link_url, :image_url, url_placeholder: { protocols: %w(http https), placeholder_regex: PLACEHOLDERS_REGEX } + validates :link_url, :image_url, url: { protocols: %w(http https) } validates :type, presence: true def rendered_link_url(project = nil) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 53af87a271a..5eb30f4aaa0 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -7,12 +7,17 @@ module Ci include Presentable include Gitlab::OptimisticLocking include Gitlab::Utils::StrongMemoize + include AtomicInternalId belongs_to :project, inverse_of: :pipelines belongs_to :user belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline' belongs_to :pipeline_schedule, class_name: 'Ci::PipelineSchedule' + has_internal_id :iid, scope: :project, presence: false, init: ->(s) do + s&.project&.pipelines&.maximum(:iid) || s&.project&.pipelines&.count + end + has_many :stages has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline @@ -531,6 +536,7 @@ module Ci def predefined_variables Gitlab::Ci::Variables::Collection.new + .append(key: 'CI_PIPELINE_IID', value: iid.to_s) .append(key: 'CI_CONFIG_PATH', value: ci_yaml_file_path) .append(key: 'CI_PIPELINE_SOURCE', value: source.to_s) .append(key: 'CI_COMMIT_MESSAGE', value: git_commit_message) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 530eacf4be0..57edd6a4956 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -12,9 +12,9 @@ module Ci FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze has_many :builds - has_many :runner_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :runner_projects, inverse_of: :runner, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :projects, through: :runner_projects - has_many :runner_namespaces + has_many :runner_namespaces, inverse_of: :runner has_many :groups, through: :runner_namespaces has_one :last_build, ->() { order('id DESC') }, class_name: 'Ci::Build' @@ -56,10 +56,15 @@ module Ci end validate :tag_constraints - validate :either_projects_or_group validates :access_level, presence: true validates :runner_type, presence: true + validate :no_projects, unless: :project_type? + validate :no_groups, unless: :group_type? + validate :any_project, if: :project_type? + validate :exactly_one_group, if: :group_type? + validate :validate_is_shared + acts_as_taggable after_destroy :cleanup_runner_queue @@ -115,8 +120,15 @@ module Ci raise ArgumentError, 'Transitioning a group runner to a project runner is not supported' end - self.save - project.runner_projects.create(runner_id: self.id) + begin + transaction do + self.projects << project + self.save! + end + rescue ActiveRecord::RecordInvalid => e + self.errors.add(:assign_to, e.message) + false + end end def display_name @@ -253,13 +265,33 @@ module Ci self.class.owned_or_shared(project_id).where(id: self.id).any? end - def either_projects_or_group - if groups.many? - errors.add(:runner, 'can only be assigned to one group') + def no_projects + if projects.any? + errors.add(:runner, 'cannot have projects assigned') + end + end + + def no_groups + if groups.any? + errors.add(:runner, 'cannot have groups assigned') + end + end + + def any_project + unless projects.any? + errors.add(:runner, 'needs to be assigned to at least one project') + end + end + + def exactly_one_group + unless groups.one? + errors.add(:runner, 'needs to be assigned to exactly one group') end + end - if assigned_to_group? && assigned_to_project? - errors.add(:runner, 'can only be assigned either to projects or to a group') + def validate_is_shared + unless is_shared? == instance_type? + errors.add(:is_shared, 'is not equal to instance_type?') end end diff --git a/app/models/ci/runner_namespace.rb b/app/models/ci/runner_namespace.rb index 3269f86e8ca..29508fdd326 100644 --- a/app/models/ci/runner_namespace.rb +++ b/app/models/ci/runner_namespace.rb @@ -2,8 +2,10 @@ module Ci class RunnerNamespace < ActiveRecord::Base extend Gitlab::Ci::Model - belongs_to :runner - belongs_to :namespace, class_name: '::Namespace' + belongs_to :runner, inverse_of: :runner_namespaces, validate: true + belongs_to :namespace, inverse_of: :runner_namespaces, class_name: '::Namespace' belongs_to :group, class_name: '::Group', foreign_key: :namespace_id + + validates :runner_id, uniqueness: { scope: :namespace_id } end end diff --git a/app/models/ci/runner_project.rb b/app/models/ci/runner_project.rb index 505d178ba8e..52437047300 100644 --- a/app/models/ci/runner_project.rb +++ b/app/models/ci/runner_project.rb @@ -2,8 +2,8 @@ module Ci class RunnerProject < ActiveRecord::Base extend Gitlab::Ci::Model - belongs_to :runner - belongs_to :project + belongs_to :runner, inverse_of: :runner_projects + belongs_to :project, inverse_of: :runner_projects validates :runner_id, uniqueness: { scope: :project_id } end diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index b881b4eaf36..e6f795f3e0b 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -43,7 +43,7 @@ module Clusters def create_and_assign_runner transaction do - project.runners.create!(runner_create_params).tap do |runner| + Ci::Runner.create!(runner_create_params).tap do |runner| update!(runner_id: runner.id) end end @@ -53,7 +53,8 @@ module Clusters { name: 'kubernetes-cluster', runner_type: :project_type, - tag_list: %w(kubernetes cluster) + tag_list: %w(kubernetes cluster), + projects: [project] } end diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 25eac5160f1..36631d57ad1 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -11,12 +11,12 @@ module Clusters attr_encrypted :password, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base, + key: Settings.attr_encrypted_db_key_base_truncated, algorithm: 'aes-256-cbc' attr_encrypted :token, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base, + key: Settings.attr_encrypted_db_key_base_truncated, algorithm: 'aes-256-cbc' before_validation :enforce_namespace_to_lower_case diff --git a/app/models/clusters/providers/gcp.rb b/app/models/clusters/providers/gcp.rb index eb2e42fd3fe..4db1bb35c12 100644 --- a/app/models/clusters/providers/gcp.rb +++ b/app/models/clusters/providers/gcp.rb @@ -11,7 +11,7 @@ module Clusters attr_encrypted :access_token, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base, + key: Settings.attr_encrypted_db_key_base_truncated, algorithm: 'aes-256-cbc' validates :gcp_project_id, diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb index 22f516a172f..164c704260e 100644 --- a/app/models/concerns/atomic_internal_id.rb +++ b/app/models/concerns/atomic_internal_id.rb @@ -25,9 +25,13 @@ module AtomicInternalId extend ActiveSupport::Concern module ClassMethods - def has_internal_id(column, scope:, init:) # rubocop:disable Naming/PredicateName - before_validation(on: :create) do + def has_internal_id(column, scope:, init:, presence: true) # rubocop:disable Naming/PredicateName + before_validation :"ensure_#{scope}_#{column}!", on: :create + validates column, presence: presence + + define_method("ensure_#{scope}_#{column}!") do scope_value = association(scope).reader + if read_attribute(column).blank? && scope_value scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value } usage = self.class.table_name.to_sym @@ -35,13 +39,9 @@ module AtomicInternalId new_iid = InternalId.generate_next(self, scope_attrs, usage, init) write_attribute(column, new_iid) end - end - validates column, presence: true, numericality: true + read_attribute(column) + end end end - - def to_param - iid.to_s - end end diff --git a/app/models/concerns/iid_routes.rb b/app/models/concerns/iid_routes.rb new file mode 100644 index 00000000000..246748cf52c --- /dev/null +++ b/app/models/concerns/iid_routes.rb @@ -0,0 +1,9 @@ +module IidRoutes + ## + # This automagically enforces all related routes to use `iid` instead of `id` + # If you want to use `iid` for some routes and `id` for other routes, this module should not to be included, + # instead you should define `iid` or `id` explictly at each route generators. e.g. pipeline_path(project.id, pipeline.iid) + def to_param + iid.to_s + end +end diff --git a/app/models/concerns/reactive_caching.rb b/app/models/concerns/reactive_caching.rb index eef9caf1c8e..be0a5b49012 100644 --- a/app/models/concerns/reactive_caching.rb +++ b/app/models/concerns/reactive_caching.rb @@ -74,6 +74,7 @@ module ReactiveCaching def clear_reactive_cache!(*args) Rails.cache.delete(full_reactive_cache_key(*args)) + Rails.cache.delete(alive_reactive_cache_key(*args)) end def exclusively_update_reactive_cache!(*args) diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb index 1caf47072bc..0fc321c52bc 100644 --- a/app/models/concerns/time_trackable.rb +++ b/app/models/concerns/time_trackable.rb @@ -30,8 +30,6 @@ module TimeTrackable return if @time_spent == 0 - touch if touchable? - if @time_spent == :reset reset_spent_time else @@ -59,10 +57,6 @@ module TimeTrackable private - def touchable? - valid? && persisted? - end - def reset_spent_time timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 254764eefde..ac86e9e8de0 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -1,5 +1,6 @@ class Deployment < ActiveRecord::Base include AtomicInternalId + include IidRoutes belongs_to :project, required: true belongs_to :environment, required: true diff --git a/app/models/environment.rb b/app/models/environment.rb index fddb269af4b..8d523dae324 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -32,7 +32,7 @@ class Environment < ActiveRecord::Base validates :external_url, length: { maximum: 255 }, allow_nil: true, - addressable_url: true + url: true delegate :stop_action, :manual_actions, to: :last_deployment, allow_nil: true diff --git a/app/models/generic_commit_status.rb b/app/models/generic_commit_status.rb index 532b8f4ad69..5ac8bde44cd 100644 --- a/app/models/generic_commit_status.rb +++ b/app/models/generic_commit_status.rb @@ -1,7 +1,7 @@ class GenericCommitStatus < CommitStatus before_validation :set_default_values - validates :target_url, addressable_url: true, + validates :target_url, url: true, length: { maximum: 255 }, allow_nil: true diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb index 0528266e5b3..6bef00f26ea 100644 --- a/app/models/hooks/system_hook.rb +++ b/app/models/hooks/system_hook.rb @@ -11,4 +11,9 @@ class SystemHook < WebHook default_value_for :push_events, false default_value_for :repository_update_events, true default_value_for :merge_requests_events, false + + # Allow urls pointing localhost and the local network + def allow_local_requests? + true + end end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 27729deeac9..e353abdda9c 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -3,7 +3,9 @@ class WebHook < ActiveRecord::Base has_many :web_hook_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - validates :url, presence: true, url: true + validates :url, presence: true, public_url: { allow_localhost: lambda(&:allow_local_requests?), + allow_local_network: lambda(&:allow_local_requests?) } + validates :token, format: { without: /\n/ } def execute(data, hook_name) @@ -13,4 +15,9 @@ class WebHook < ActiveRecord::Base def async_execute(data, hook_name) WebHookService.new(self, data, hook_name).async_execute end + + # Allow urls pointing localhost and the local network + def allow_local_requests? + false + end end diff --git a/app/models/internal_id.rb b/app/models/internal_id.rb index f7f930e86ed..f50f28deffe 100644 --- a/app/models/internal_id.rb +++ b/app/models/internal_id.rb @@ -14,7 +14,7 @@ class InternalId < ActiveRecord::Base belongs_to :project belongs_to :namespace - enum usage: { issues: 0, merge_requests: 1, deployments: 2, milestones: 3, epics: 4 } + enum usage: { issues: 0, merge_requests: 1, deployments: 2, milestones: 3, epics: 4, ci_pipelines: 5 } validates :usage, presence: true diff --git a/app/models/issue.rb b/app/models/issue.rb index 41a290f34b4..d136700836d 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -2,6 +2,7 @@ require 'carrierwave/orm/activerecord' class Issue < ActiveRecord::Base include AtomicInternalId + include IidRoutes include Issuable include Noteable include Referable diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 79fc155fd3c..4c1628d2bdb 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1,5 +1,6 @@ class MergeRequest < ActiveRecord::Base include AtomicInternalId + include IidRoutes include Issuable include Noteable include Referable diff --git a/app/models/milestone.rb b/app/models/milestone.rb index d14e3a4ded5..d05dcfd083a 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -9,6 +9,7 @@ class Milestone < ActiveRecord::Base include CacheMarkdownField include AtomicInternalId + include IidRoutes include Sortable include Referable include StripAttribute diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 3dad4277713..52fe529c016 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -21,7 +21,7 @@ class Namespace < ActiveRecord::Base has_many :projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :project_statistics - has_many :runner_namespaces, class_name: 'Ci::RunnerNamespace' + has_many :runner_namespaces, inverse_of: :namespace, class_name: 'Ci::RunnerNamespace' has_many :runners, through: :runner_namespaces, source: :runner, class_name: 'Ci::Runner' # This should _not_ be `inverse_of: :namespace`, because that would also set diff --git a/app/models/project.rb b/app/models/project.rb index e275ac4dc6f..32298fc7f5c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -236,7 +236,7 @@ class Project < ActiveRecord::Base has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :build_trace_section_names, class_name: 'Ci::BuildTraceSectionName' has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks - has_many :runner_projects, class_name: 'Ci::RunnerProject' + has_many :runner_projects, class_name: 'Ci::RunnerProject', inverse_of: :project has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner' has_many :variables, class_name: 'Ci::Variable' has_many :triggers, class_name: 'Ci::Trigger' @@ -289,8 +289,9 @@ class Project < ActiveRecord::Base validates :namespace, presence: true validates :name, uniqueness: { scope: :namespace_id } - validates :import_url, addressable_url: true, if: :external_import? - validates :import_url, importable_url: true, if: [:external_import?, :import_url_changed?] + validates :import_url, url: { protocols: %w(http https ssh git), + allow_localhost: false, + ports: VALID_IMPORT_PORTS }, if: [:external_import?, :import_url_changed?] validates :star_count, numericality: { greater_than_or_equal_to: 0 } validate :check_limit, on: :create validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? } diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb index 54e4b3278db..7f4c47a6d14 100644 --- a/app/models/project_services/bamboo_service.rb +++ b/app/models/project_services/bamboo_service.rb @@ -3,7 +3,7 @@ class BambooService < CiService prop_accessor :bamboo_url, :build_key, :username, :password - validates :bamboo_url, presence: true, url: true, if: :activated? + validates :bamboo_url, presence: true, public_url: true, if: :activated? validates :build_key, presence: true, if: :activated? validates :username, presence: true, diff --git a/app/models/project_services/bugzilla_service.rb b/app/models/project_services/bugzilla_service.rb index 046e2809f45..e4e3a80976b 100644 --- a/app/models/project_services/bugzilla_service.rb +++ b/app/models/project_services/bugzilla_service.rb @@ -1,5 +1,5 @@ class BugzillaService < IssueTrackerService - validates :project_url, :issues_url, :new_issue_url, presence: true, url: true, if: :activated? + validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated? prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb index d2aaff8817a..35884c4560c 100644 --- a/app/models/project_services/buildkite_service.rb +++ b/app/models/project_services/buildkite_service.rb @@ -8,7 +8,7 @@ class BuildkiteService < CiService prop_accessor :project_url, :token boolean_accessor :enable_ssl_verification - validates :project_url, presence: true, url: true, if: :activated? + validates :project_url, presence: true, public_url: true, if: :activated? validates :token, presence: true, if: :activated? after_save :compose_service_hook, if: :activated? diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index 7591ab4f478..ae0debbd3ac 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -8,7 +8,7 @@ class ChatNotificationService < Service prop_accessor :webhook, :username, :channel boolean_accessor :notify_only_broken_pipelines, :notify_only_default_branch - validates :webhook, presence: true, url: true, if: :activated? + validates :webhook, presence: true, public_url: true, if: :activated? def initialize_properties # Custom serialized properties initialization diff --git a/app/models/project_services/custom_issue_tracker_service.rb b/app/models/project_services/custom_issue_tracker_service.rb index b9e3e982b64..456c7f5cee2 100644 --- a/app/models/project_services/custom_issue_tracker_service.rb +++ b/app/models/project_services/custom_issue_tracker_service.rb @@ -1,5 +1,5 @@ class CustomIssueTrackerService < IssueTrackerService - validates :project_url, :issues_url, :new_issue_url, presence: true, url: true, if: :activated? + validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated? prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb index a4bf427ac0b..ab4e46da89f 100644 --- a/app/models/project_services/drone_ci_service.rb +++ b/app/models/project_services/drone_ci_service.rb @@ -4,7 +4,7 @@ class DroneCiService < CiService prop_accessor :drone_url, :token boolean_accessor :enable_ssl_verification - validates :drone_url, presence: true, url: true, if: :activated? + validates :drone_url, presence: true, public_url: true, if: :activated? validates :token, presence: true, if: :activated? after_save :compose_service_hook, if: :activated? diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb index 1553f169827..a4b1ef09e93 100644 --- a/app/models/project_services/external_wiki_service.rb +++ b/app/models/project_services/external_wiki_service.rb @@ -1,7 +1,7 @@ class ExternalWikiService < Service prop_accessor :external_wiki_url - validates :external_wiki_url, presence: true, url: true, if: :activated? + validates :external_wiki_url, presence: true, public_url: true, if: :activated? def title 'External Wiki' diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb index 88c428b4aae..16e32a4139e 100644 --- a/app/models/project_services/gitlab_issue_tracker_service.rb +++ b/app/models/project_services/gitlab_issue_tracker_service.rb @@ -1,7 +1,7 @@ class GitlabIssueTrackerService < IssueTrackerService include Gitlab::Routing - validates :project_url, :issues_url, :new_issue_url, presence: true, url: true, if: :activated? + validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated? prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index ed4bbfb6cfc..eb3261c902f 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -3,8 +3,8 @@ class JiraService < IssueTrackerService include ApplicationHelper include ActionView::Helpers::AssetUrlHelper - validates :url, url: true, presence: true, if: :activated? - validates :api_url, url: true, allow_blank: true + validates :url, public_url: true, presence: true, if: :activated? + validates :api_url, public_url: true, allow_blank: true validates :username, presence: true, if: :activated? validates :password, presence: true, if: :activated? diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index 20fed432e55..ddd4026019b 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -24,7 +24,7 @@ class KubernetesService < DeploymentService prop_accessor :ca_pem with_options presence: true, if: :activated? do - validates :api_url, url: true + validates :api_url, public_url: true validates :token end diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb index 2221459c90b..b89dc07a73e 100644 --- a/app/models/project_services/mock_ci_service.rb +++ b/app/models/project_services/mock_ci_service.rb @@ -3,7 +3,7 @@ class MockCiService < CiService ALLOWED_STATES = %w[failed canceled running pending success success_with_warnings skipped not_found].freeze prop_accessor :mock_service_url - validates :mock_service_url, presence: true, url: true, if: :activated? + validates :mock_service_url, presence: true, public_url: true, if: :activated? def title 'MockCI' diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index dcaeb65dc32..df4254e0523 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -6,7 +6,7 @@ class PrometheusService < MonitoringService boolean_accessor :manual_configuration with_options presence: true, if: :manual_configuration? do - validates :api_url, url: true + validates :api_url, public_url: true end before_save :synchronize_service_state diff --git a/app/models/project_services/redmine_service.rb b/app/models/project_services/redmine_service.rb index 6acf611eba5..3721093a6d1 100644 --- a/app/models/project_services/redmine_service.rb +++ b/app/models/project_services/redmine_service.rb @@ -1,5 +1,5 @@ class RedmineService < IssueTrackerService - validates :project_url, :issues_url, :new_issue_url, presence: true, url: true, if: :activated? + validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated? prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb index 145313b8e71..802678147cf 100644 --- a/app/models/project_services/teamcity_service.rb +++ b/app/models/project_services/teamcity_service.rb @@ -3,7 +3,7 @@ class TeamcityService < CiService prop_accessor :teamcity_url, :build_type, :username, :password - validates :teamcity_url, presence: true, url: true, if: :activated? + validates :teamcity_url, presence: true, public_url: true, if: :activated? validates :build_type, presence: true, if: :activated? validates :username, presence: true, diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index f799a0b4227..a6f94b3e3b0 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -140,10 +140,6 @@ class ProjectWiki [title, title_array.join("/")] end - def search_files(query) - repository.search_files_by_content(query, default_branch) - end - def repository @repository ||= Repository.new(full_path, @project, disk_path: disk_path, is_wiki: true) end diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index aba1f2f384f..5cd222e18a4 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -17,7 +17,6 @@ class RemoteMirror < ActiveRecord::Base belongs_to :project, inverse_of: :remote_mirrors validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true } - validates :url, addressable_url: true, if: :url_changed? before_save :set_new_remote_name, if: :mirror_url_changed? diff --git a/app/models/timelog.rb b/app/models/timelog.rb index e166cf69703..f4c5c581a11 100644 --- a/app/models/timelog.rb +++ b/app/models/timelog.rb @@ -2,8 +2,8 @@ class Timelog < ActiveRecord::Base validates :time_spent, :user, presence: true validate :issuable_id_is_present - belongs_to :issue - belongs_to :merge_request + belongs_to :issue, touch: true + belongs_to :merge_request, touch: true belongs_to :user def issuable diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 1f67e3ecf9d..683f64e82ad 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -183,7 +183,10 @@ class IssuableBaseService < BaseService old_associations = associations_before_update(issuable) label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) - params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids) + if labels_changing?(issuable.label_ids, label_ids) + params[:label_ids] = label_ids + issuable.touch + end if issuable.changed? || params.present? issuable.assign_attributes(params.merge(updated_by: current_user)) diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index bdd9598f85a..00080717600 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -29,7 +29,7 @@ module Projects def add_repository_to_project if project.external_import? && !unknown_url? begin - Gitlab::UrlBlocker.validate!(project.import_url, valid_ports: Project::VALID_IMPORT_PORTS) + Gitlab::UrlBlocker.validate!(project.import_url, ports: Project::VALID_IMPORT_PORTS) rescue Gitlab::UrlBlocker::BlockedUrlError => e raise Error, "Blocked import URL: #{e.message}" end diff --git a/app/validators/addressable_url_validator.rb b/app/validators/addressable_url_validator.rb deleted file mode 100644 index 94542125d43..00000000000 --- a/app/validators/addressable_url_validator.rb +++ /dev/null @@ -1,45 +0,0 @@ -# AddressableUrlValidator -# -# Custom validator for URLs. This is a stricter version of UrlValidator - it also checks -# for using the right protocol, but it actually parses the URL checking for any syntax errors. -# The regex is also different from `URI` as we use `Addressable::URI` here. -# -# By default, only URLs for http, https, ssh, and git protocols will be considered valid. -# Provide a `:protocols` option to configure accepted protocols. -# -# Example: -# -# class User < ActiveRecord::Base -# validates :personal_url, addressable_url: true -# -# validates :ftp_url, addressable_url: { protocols: %w(ftp) } -# -# validates :git_url, addressable_url: { protocols: %w(http https ssh git) } -# end -# -class AddressableUrlValidator < ActiveModel::EachValidator - DEFAULT_OPTIONS = { protocols: %w(http https ssh git) }.freeze - - def validate_each(record, attribute, value) - unless valid_url?(value) - record.errors.add(attribute, "must be a valid URL") - end - end - - private - - def valid_url?(value) - return false unless value - - valid_protocol?(value) && valid_uri?(value) - end - - def valid_uri?(value) - Gitlab::UrlSanitizer.valid?(value) - end - - def valid_protocol?(value) - options = DEFAULT_OPTIONS.merge(self.options) - value =~ /\A#{URI.regexp(options[:protocols])}\z/ - end -end diff --git a/app/validators/importable_url_validator.rb b/app/validators/importable_url_validator.rb deleted file mode 100644 index 612d3c71913..00000000000 --- a/app/validators/importable_url_validator.rb +++ /dev/null @@ -1,11 +0,0 @@ -# ImportableUrlValidator -# -# This validator blocks projects from using dangerous import_urls to help -# protect against Server-side Request Forgery (SSRF). -class ImportableUrlValidator < ActiveModel::EachValidator - def validate_each(record, attribute, value) - Gitlab::UrlBlocker.validate!(value, valid_ports: Project::VALID_IMPORT_PORTS) - rescue Gitlab::UrlBlocker::BlockedUrlError => e - record.errors.add(attribute, "is blocked: #{e.message}") - end -end diff --git a/app/validators/public_url_validator.rb b/app/validators/public_url_validator.rb new file mode 100644 index 00000000000..1e8118fccbb --- /dev/null +++ b/app/validators/public_url_validator.rb @@ -0,0 +1,26 @@ +# PublicUrlValidator +# +# Custom validator for URLs. This validator works like UrlValidator but +# it blocks by default urls pointing to localhost or the local network. +# +# This validator accepts the same params UrlValidator does. +# +# Example: +# +# class User < ActiveRecord::Base +# validates :personal_url, public_url: true +# +# validates :ftp_url, public_url: { protocols: %w(ftp) } +# +# validates :git_url, public_url: { allow_localhost: true, allow_local_network: true} +# end +# +class PublicUrlValidator < UrlValidator + private + + def default_options + # By default block all urls pointing to localhost or the local network + super.merge(allow_localhost: false, + allow_local_network: false) + end +end diff --git a/app/validators/url_placeholder_validator.rb b/app/validators/url_placeholder_validator.rb deleted file mode 100644 index dd681218b6b..00000000000 --- a/app/validators/url_placeholder_validator.rb +++ /dev/null @@ -1,32 +0,0 @@ -# UrlValidator -# -# Custom validator for URLs. -# -# By default, only URLs for the HTTP(S) protocols will be considered valid. -# Provide a `:protocols` option to configure accepted protocols. -# -# Also, this validator can help you validate urls with placeholders inside. -# Usually, if you have a url like 'http://www.example.com/%{project_path}' the -# URI parser will reject that URL format. Provide a `:placeholder_regex` option -# to configure accepted placeholders. -# -# Example: -# -# class User < ActiveRecord::Base -# validates :personal_url, url: true -# -# validates :ftp_url, url: { protocols: %w(ftp) } -# -# validates :git_url, url: { protocols: %w(http https ssh git) } -# -# validates :placeholder_url, url: { placeholder_regex: /(project_path|project_id|default_branch)/ } -# end -# -class UrlPlaceholderValidator < UrlValidator - def validate_each(record, attribute, value) - placeholder_regex = self.options[:placeholder_regex] - value = value.gsub(/%{#{placeholder_regex}}/, 'foo') if placeholder_regex && value - - super(record, attribute, value) - end -end diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb index a77beb2683d..8648c4c75e3 100644 --- a/app/validators/url_validator.rb +++ b/app/validators/url_validator.rb @@ -15,25 +15,63 @@ # validates :git_url, url: { protocols: %w(http https ssh git) } # end # +# This validator can also block urls pointing to localhost or the local network to +# protect against Server-side Request Forgery (SSRF), or check for the right port. +# +# Example: +# class User < ActiveRecord::Base +# validates :personal_url, url: { allow_localhost: false, allow_local_network: false} +# +# validates :web_url, url: { ports: [80, 443] } +# end class UrlValidator < ActiveModel::EachValidator + DEFAULT_PROTOCOLS = %w(http https).freeze + + attr_reader :record + def validate_each(record, attribute, value) - unless valid_url?(value) + @record = record + + if value.present? + value.strip! + else record.errors.add(attribute, "must be a valid URL") end + + Gitlab::UrlBlocker.validate!(value, blocker_args) + rescue Gitlab::UrlBlocker::BlockedUrlError => e + record.errors.add(attribute, "is blocked: #{e.message}") end private def default_options - @default_options ||= { protocols: %w(http https) } + # By default the validator doesn't block any url based on the ip address + { + protocols: DEFAULT_PROTOCOLS, + ports: [], + allow_localhost: true, + allow_local_network: true + } end - def valid_url?(value) - return false if value.nil? + def current_options + options = self.options.map do |option, value| + [option, value.is_a?(Proc) ? value.call(record) : value] + end.to_h + + default_options.merge(options) + end - options = default_options.merge(self.options) + def blocker_args + current_options.slice(:allow_localhost, :allow_local_network, :protocols, :ports).tap do |args| + if allow_setting_local_requests? + args[:allow_localhost] = args[:allow_local_network] = true + end + end + end - value.strip! - value =~ /\A#{URI.regexp(options[:protocols])}\z/ + def allow_setting_local_requests? + ApplicationSetting.current&.allow_local_requests_from_hooks_and_services? end end diff --git a/app/views/admin/labels/_form.html.haml b/app/views/admin/labels/_form.html.haml index ee51b44e83e..7637471f9ae 100644 --- a/app/views/admin/labels/_form.html.haml +++ b/app/views/admin/labels/_form.html.haml @@ -19,7 +19,7 @@ .form-text.text-muted Choose any color. %br - Or you can choose one of suggested colors below + Or you can choose one of the suggested colors below .suggest-colors - suggested_colors.each do |color| diff --git a/app/views/admin/logs/show.html.haml b/app/views/admin/logs/show.html.haml index a6c436cd1f4..e4c0382a437 100644 --- a/app/views/admin/logs/show.html.haml +++ b/app/views/admin/logs/show.html.haml @@ -4,8 +4,8 @@ %div{ class: container_class } %ul.nav-links.log-tabs.nav.nav-tabs - @loggers.each do |klass| - %li{ class: active_when(klass == @loggers.first) }> - = link_to klass.file_name, "##{klass.file_name_noext}", data: { toggle: 'tab' } + %li.nav-item + = link_to klass.file_name, "##{klass.file_name_noext}", data: { toggle: 'tab' }, class: "#{active_when(klass == @loggers.first)} nav-link" .row-content-block To prevent performance issues admin logs output the last 2000 lines .tab-content diff --git a/app/views/help/_shortcuts.html.haml b/app/views/help/_shortcuts.html.haml index 9a3a03a7671..29db29235c1 100644 --- a/app/views/help/_shortcuts.html.haml +++ b/app/views/help/_shortcuts.html.haml @@ -2,11 +2,12 @@ .modal-dialog.modal-lg .modal-content .modal-header - %a.close{ href: "#", "data-dismiss" => "modal" } × - %h4 + %h4.modal-title Keyboard Shortcuts %small = link_to '(Show all)', '#', class: 'js-more-help-button' + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body .row .col-lg-4 diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index ab5565cfdaf..ce312943154 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -4,7 +4,7 @@ = form_for @user, url: profile_preferences_path, remote: true, method: :put, html: { class: 'row prepend-top-default js-preferences-form' } do |f| .col-lg-4.application-theme %h4.prepend-top-0 - GitLab navigation theme + s_('Preferences|Navigation theme') %p Customize the appearance of the application header and navigation sidebar. .col-lg-8.application-theme - Gitlab::Themes.each do |theme| diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index fbb29e7a0d9..507cd5dcc12 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -77,11 +77,10 @@ .modal-dialog .modal-content .modal-header - %button.close{ type: 'button', 'data-dismiss': 'modal' } - %span - × %h4.modal-title Position and size your new avatar + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body .profile-crop-image-container %img.modal-profile-crop-image{ alt: 'Avatar cropper' } diff --git a/app/views/projects/_bitbucket_import_modal.html.haml b/app/views/projects/_bitbucket_import_modal.html.haml index c24a496486c..c54a4ceb890 100644 --- a/app/views/projects/_bitbucket_import_modal.html.haml +++ b/app/views/projects/_bitbucket_import_modal.html.haml @@ -2,8 +2,9 @@ .modal-dialog .modal-content .modal-header - %a.close{ href: "#", "data-dismiss" => "modal" } × - %h3 Import projects from Bitbucket + %h3.modal-title Import projects from Bitbucket + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body To enable importing projects from Bitbucket, - if current_user.admin? diff --git a/app/views/projects/_gitlab_import_modal.html.haml b/app/views/projects/_gitlab_import_modal.html.haml index 00aef66e1f8..5519415cdc3 100644 --- a/app/views/projects/_gitlab_import_modal.html.haml +++ b/app/views/projects/_gitlab_import_modal.html.haml @@ -2,8 +2,9 @@ .modal-dialog .modal-content .modal-header - %a.close{ href: "#", "data-dismiss" => "modal" } × - %h3 Import projects from GitLab.com + %h3.modal-title Import projects from GitLab.com + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body To enable importing projects from GitLab.com, - if current_user.admin? diff --git a/app/views/projects/_issuable_by_email.html.haml b/app/views/projects/_issuable_by_email.html.haml index e3dc0677bd6..22adf5b4008 100644 --- a/app/views/projects/_issuable_by_email.html.haml +++ b/app/views/projects/_issuable_by_email.html.haml @@ -8,10 +8,10 @@ .modal-dialog{ role: "document" } .modal-content .modal-header - %button.close{ type: "button", data: { dismiss: "modal" }, aria: { label: "close" } } - %span{ aria: { hidden: "true" } }= icon("times") %h4.modal-title Create new #{name} by email + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body %p You can create a new #{name} inside this project by sending an email to the following email address: diff --git a/app/views/projects/blob/_new_dir.html.haml b/app/views/projects/blob/_new_dir.html.haml index e7a4e3d67cb..6f3a691518b 100644 --- a/app/views/projects/blob/_new_dir.html.haml +++ b/app/views/projects/blob/_new_dir.html.haml @@ -2,8 +2,9 @@ .modal-dialog.modal-lg .modal-content .modal-header - %a.close{ href: "#", "data-dismiss" => "modal" } × %h3.page-title= _('Create New Directory') + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body = form_tag project_create_dir_path(@project, @id), method: :post, remote: false, class: 'js-create-dir-form js-quick-submit js-requires-input' do .form-group.row diff --git a/app/views/projects/blob/_remove.html.haml b/app/views/projects/blob/_remove.html.haml index 4628ecff3d6..f80bae5c88c 100644 --- a/app/views/projects/blob/_remove.html.haml +++ b/app/views/projects/blob/_remove.html.haml @@ -2,8 +2,9 @@ .modal-dialog .modal-content .modal-header - %a.close{ href: "#", "data-dismiss" => "modal" } × %h3.page-title Delete #{@blob.name} + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body = form_tag project_blob_path(@project, @id), method: :delete, class: 'js-delete-blob-form js-quick-submit js-requires-input' do diff --git a/app/views/projects/blob/_upload.html.haml b/app/views/projects/blob/_upload.html.haml index 60a49441ce8..0a5c73c9037 100644 --- a/app/views/projects/blob/_upload.html.haml +++ b/app/views/projects/blob/_upload.html.haml @@ -2,8 +2,9 @@ .modal-dialog.modal-lg .modal-content .modal-header - %a.close{ href: "#", "data-dismiss" => "modal" } × %h3.page-title= title + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body = form_tag form_path, method: method, class: 'js-quick-submit js-upload-blob-form', data: { method: method } do .dropzone diff --git a/app/views/projects/branches/_delete_protected_modal.html.haml b/app/views/projects/branches/_delete_protected_modal.html.haml index e0008e322a0..8aa79d2d464 100644 --- a/app/views/projects/branches/_delete_protected_modal.html.haml +++ b/app/views/projects/branches/_delete_protected_modal.html.haml @@ -2,11 +2,12 @@ .modal-dialog .modal-content .modal-header - %button.close{ data: { dismiss: 'modal' } } × %h3.page-title - title_branch_name = capture do %span.js-branch-name.ref-name>[branch name] = s_("Branches|Delete protected branch '%{branch_name}'?").html_safe % { branch_name: title_branch_name } + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body %p diff --git a/app/views/projects/commit/_change.html.haml b/app/views/projects/commit/_change.html.haml index 430bc8f59f9..30605927fd1 100644 --- a/app/views/projects/commit/_change.html.haml +++ b/app/views/projects/commit/_change.html.haml @@ -15,8 +15,9 @@ .modal-dialog .modal-content .modal-header - %a.close{ href: "#", "data-dismiss" => "modal" } × %h3.page-title= title + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body - if description %p.append-bottom-20= description diff --git a/app/views/projects/deploy_tokens/_revoke_modal.html.haml b/app/views/projects/deploy_tokens/_revoke_modal.html.haml index ace3480c815..a67c3a0c841 100644 --- a/app/views/projects/deploy_tokens/_revoke_modal.html.haml +++ b/app/views/projects/deploy_tokens/_revoke_modal.html.haml @@ -2,11 +2,11 @@ .modal-dialog .modal-content .modal-header - %h4.modal-title.float-left + %h4.modal-title = s_('DeployTokens|Revoke') %b #{token.name}? - %button.close{ 'aria-label' => _('Close'), 'data-dismiss' => 'modal', type: 'button' } - %span{ 'aria-hidden' => 'true' } × + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body %p = s_('DeployTokens|You are about to revoke') diff --git a/app/views/projects/merge_requests/_how_to_merge.html.haml b/app/views/projects/merge_requests/_how_to_merge.html.haml index 5353fa8a88f..62dd21ef6e0 100644 --- a/app/views/projects/merge_requests/_how_to_merge.html.haml +++ b/app/views/projects/merge_requests/_how_to_merge.html.haml @@ -2,8 +2,9 @@ .modal-dialog .modal-content .modal-header - %h3 Check out, review, and merge locally - %a.close{ href: "#", "data-dismiss" => "modal" } × + %h3.modal-title Check out, review, and merge locally + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body %p %strong Step 1. diff --git a/app/views/projects/wikis/_new.html.haml b/app/views/projects/wikis/_new.html.haml index 06a3cac12d5..38382aae67c 100644 --- a/app/views/projects/wikis/_new.html.haml +++ b/app/views/projects/wikis/_new.html.haml @@ -2,8 +2,9 @@ .modal-dialog .modal-content .modal-header - %a.close{ href: "#", "data-dismiss" => "modal" } × %h3.page-title= s_("WikiNewPageTitle|New Wiki Page") + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body %form.new-wiki-page .form-group diff --git a/app/views/search/results/_blob.html.haml b/app/views/search/results/_blob.html.haml index de473c23d66..fdcd126e7a3 100644 --- a/app/views/search/results/_blob.html.haml +++ b/app/views/search/results/_blob.html.haml @@ -1,13 +1,5 @@ -- file_name, blob = blob -.blob-result - .file-holder - .js-file-title.file-title - - ref = @search_results.repository_ref - - blob_link = project_blob_path(@project, tree_join(ref, file_name)) - = link_to blob_link do - %i.fa.fa-file - %strong - = file_name - - if blob - .file-content.code.term - = render 'shared/file_highlight', blob: blob, first_line_number: blob.startline, blob_link: blob_link +- project = find_project_for_result_blob(blob) +- file_name, blob = parse_search_result(blob) +- blob_link = project_blob_path(project, tree_join(blob.ref, file_name)) + += render partial: 'search/results/blob_data', locals: { blob: blob, project: project, file_name: file_name, blob_link: blob_link } diff --git a/app/views/search/results/_blob_data.html.haml b/app/views/search/results/_blob_data.html.haml new file mode 100644 index 00000000000..0115be41ff1 --- /dev/null +++ b/app/views/search/results/_blob_data.html.haml @@ -0,0 +1,9 @@ +.blob-result + .file-holder + .js-file-title.file-title + = link_to blob_link do + %i.fa.fa-file + = search_blob_title(project, file_name) + - if blob.data + .file-content.code.term + = render 'shared/file_highlight', blob: blob, first_line_number: blob.startline diff --git a/app/views/search/results/_wiki_blob.html.haml b/app/views/search/results/_wiki_blob.html.haml index 16a0e432d62..4346217c230 100644 --- a/app/views/search/results/_wiki_blob.html.haml +++ b/app/views/search/results/_wiki_blob.html.haml @@ -1,10 +1,5 @@ -- wiki_blob = parse_search_result(wiki_blob) -.blob-result - .file-holder - .js-file-title.file-title - = link_to project_wiki_path(@project, wiki_blob.basename) do - %i.fa.fa-file - %strong - = wiki_blob.basename - .file-content.code.term - = render 'shared/file_highlight', blob: wiki_blob, first_line_number: wiki_blob.startline +- project = find_project_for_result_blob(wiki_blob) +- file_name, wiki_blob = parse_search_result(wiki_blob) +- wiki_blob_link = project_wiki_path(project, wiki_blob.basename) + += render partial: 'search/results/blob_data', locals: { blob: wiki_blob, project: project, file_name: file_name, blob_link: wiki_blob_link } diff --git a/app/views/shared/_confirm_modal.html.haml b/app/views/shared/_confirm_modal.html.haml index 7c326d36d99..1dcf4369253 100644 --- a/app/views/shared/_confirm_modal.html.haml +++ b/app/views/shared/_confirm_modal.html.haml @@ -4,7 +4,8 @@ .modal-header %h3.page-title Confirmation required - %a.close{ href: "#", "data-dismiss" => "modal" } × + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body %p.text-danger.js-confirm-text diff --git a/app/views/shared/_delete_label_modal.html.haml b/app/views/shared/_delete_label_modal.html.haml index 01effefc34d..b96380923ac 100644 --- a/app/views/shared/_delete_label_modal.html.haml +++ b/app/views/shared/_delete_label_modal.html.haml @@ -2,8 +2,9 @@ .modal-dialog .modal-content .modal-header - %button.close{ data: {dismiss: 'modal' } } × %h3.page-title Delete #{render_colored_label(label, tooltip: false)} ? + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body %p diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 8497e3220ee..b49e47a7266 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -29,6 +29,8 @@ = render 'shared/issuable/form/metadata', issuable: issuable, form: form += render_if_exists 'shared/issuable/approvals', issuable: issuable, form: form + = render 'shared/issuable/form/branch_chooser', issuable: issuable, form: form = render 'shared/issuable/form/merge_params', issuable: issuable @@ -77,5 +79,6 @@ %strong= link_to('contribution guidelines', guide_url) for this project. += render_if_exists 'shared/issuable/remove_approver' = form.hidden_field :lock_version diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index a57cd4b20d1..9e50e888b35 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -18,6 +18,9 @@ = render "shared/issuable/sidebar_todo", todo: todo, issuable: issuable, is_collapsed: true .block.assignee = render "shared/issuable/sidebar_assignees", issuable: issuable, can_edit_issuable: can_edit_issuable, signed_in: current_user.present? + + = render_if_exists 'shared/issuable/sidebar_item_epic', issuable: issuable + .block.milestone .sidebar-collapsed-icon.has-tooltip{ title: milestone_tooltip_title(issuable.milestone), data: { container: 'body', html: 'true', placement: 'left', boundary: 'viewport' } } = icon('clock-o', 'aria-hidden': 'true') @@ -115,6 +118,8 @@ - if can? current_user, :admin_label, @project and @project = render partial: "shared/issuable/label_page_create" + = render_if_exists 'shared/issuable/sidebar_weight', issuable: issuable + - if issuable.has_attribute?(:confidential) -# haml-lint:disable InlineJavaScript %script#js-confidential-issue-data{ type: "application/json" }= { is_confidential: @issue.confidential, is_editable: can_edit_issuable }.to_json.html_safe @@ -139,7 +144,7 @@ = _('Reference:') %cite{ title: project_ref } = project_ref - = clipboard_button(text: project_ref, title: _('Copy reference to clipboard'), placement: "left") + = clipboard_button(text: project_ref, title: _('Copy reference to clipboard'), placement: "left", boundary: 'viewport') - if current_user && issuable.can_move?(current_user) .block.js-sidebar-move-issue-block .sidebar-collapsed-icon{ data: { toggle: 'tooltip', placement: 'left', container: 'body', boundary: 'viewport' }, title: _('Move issue') } diff --git a/app/views/shared/issuable/form/_merge_params.html.haml b/app/views/shared/issuable/form/_merge_params.html.haml index 1bcdddd0df0..1881875b7c0 100644 --- a/app/views/shared/issuable/form/_merge_params.html.haml +++ b/app/views/shared/issuable/form/_merge_params.html.haml @@ -3,13 +3,9 @@ - return unless issuable.is_a?(MergeRequest) - return if issuable.closed_without_fork? --# This check is duplicated below to avoid CE -> EE merge conflicts. --# This comment and the following line should only exist in CE. -- return unless issuable.can_remove_source_branch?(current_user) - -.form-group.row - .col-sm-10.offset-sm-2 - - if issuable.can_remove_source_branch?(current_user) +- if issuable.can_remove_source_branch?(current_user) + .form-group.row + .col-sm-10.offset-sm-2 .form-check = hidden_field_tag 'merge_request[force_remove_source_branch]', '0', id: nil = check_box_tag 'merge_request[force_remove_source_branch]', '1', issuable.force_remove_source_branch?, class: 'form-check-input' diff --git a/app/views/shared/labels/_form.html.haml b/app/views/shared/labels/_form.html.haml index f79f66b144f..2bf5efae1e6 100644 --- a/app/views/shared/labels/_form.html.haml +++ b/app/views/shared/labels/_form.html.haml @@ -19,7 +19,7 @@ .form-text.text-muted Choose any color. %br - Or you can choose one of suggested colors below + Or you can choose one of the suggested colors below .suggest-colors - suggested_colors.each do |color| diff --git a/app/views/shared/notifications/_custom_notifications.html.haml b/app/views/shared/notifications/_custom_notifications.html.haml index 51d912b4a66..1f6e8f98bbb 100644 --- a/app/views/shared/notifications/_custom_notifications.html.haml +++ b/app/views/shared/notifications/_custom_notifications.html.haml @@ -2,10 +2,10 @@ .modal-dialog .modal-content .modal-header - %button.close{ type: "button", "aria-label": "close", data: { dismiss: "modal" } } - %span{ "aria-hidden": "true" } × %h4#custom-notifications-title.modal-title #{ _('Custom notification events') } + %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } + %span{ "aria-hidden": true } × .modal-body .container-fluid diff --git a/changelogs/unreleased/46478-update-updated-at-on-mr.yml b/changelogs/unreleased/46478-update-updated-at-on-mr.yml new file mode 100644 index 00000000000..c58b4fc8f84 --- /dev/null +++ b/changelogs/unreleased/46478-update-updated-at-on-mr.yml @@ -0,0 +1,5 @@ +--- +title: Updates updated_at on label changes +merge_request: 19065 +author: Jacopo Beschi @jacopo-beschi +type: fixed diff --git a/changelogs/unreleased/47113-modal-header-styling-is-broken.yml b/changelogs/unreleased/47113-modal-header-styling-is-broken.yml new file mode 100644 index 00000000000..1c78e5d4211 --- /dev/null +++ b/changelogs/unreleased/47113-modal-header-styling-is-broken.yml @@ -0,0 +1,5 @@ +--- +title: Fixes the styling on the modal headers +merge_request: 19312 +author: samdbeckham +type: fixed diff --git a/changelogs/unreleased/47183-update-selenium-webdriver-to-3-12-0.yml b/changelogs/unreleased/47183-update-selenium-webdriver-to-3-12-0.yml new file mode 100644 index 00000000000..b0d51d810f2 --- /dev/null +++ b/changelogs/unreleased/47183-update-selenium-webdriver-to-3-12-0.yml @@ -0,0 +1,5 @@ +--- +title: Update selenium-webdriver to 3.12.0 +merge_request: 19351 +author: Takuya Noguchi +type: other diff --git a/changelogs/unreleased/bvl-bump-gitlab-shell-7-1-3.yml b/changelogs/unreleased/bvl-bump-gitlab-shell-7-1-3.yml new file mode 100644 index 00000000000..76bb25bc7d7 --- /dev/null +++ b/changelogs/unreleased/bvl-bump-gitlab-shell-7-1-3.yml @@ -0,0 +1,5 @@ +--- +title: Include username in output when testing SSH to GitLab +merge_request: 19358 +author: +type: other diff --git a/changelogs/unreleased/fix-http-proxy.yml b/changelogs/unreleased/fix-http-proxy.yml new file mode 100644 index 00000000000..806b7d0a38c --- /dev/null +++ b/changelogs/unreleased/fix-http-proxy.yml @@ -0,0 +1,5 @@ +--- +title: Fixed HTTP_PROXY environment not honored when reading remote traces. +merge_request: 19282 +author: NLR +type: fixed diff --git a/changelogs/unreleased/fj-34526-enabling-wiki-search-by-title.yml b/changelogs/unreleased/fj-34526-enabling-wiki-search-by-title.yml new file mode 100644 index 00000000000..2ae2cf8a23e --- /dev/null +++ b/changelogs/unreleased/fj-34526-enabling-wiki-search-by-title.yml @@ -0,0 +1,5 @@ +--- +title: Added ability to search by wiki titles +merge_request: 19112 +author: +type: added diff --git a/changelogs/unreleased/fj-45059-add-validation-to-webhook.yml b/changelogs/unreleased/fj-45059-add-validation-to-webhook.yml new file mode 100644 index 00000000000..e9350cc7e7e --- /dev/null +++ b/changelogs/unreleased/fj-45059-add-validation-to-webhook.yml @@ -0,0 +1,5 @@ +--- +title: Refactoring UrlValidators to include url blocking +merge_request: 18686 +author: +type: changed diff --git a/changelogs/unreleased/new-label-spelling-error.yml b/changelogs/unreleased/new-label-spelling-error.yml new file mode 100644 index 00000000000..ad5f69688f3 --- /dev/null +++ b/changelogs/unreleased/new-label-spelling-error.yml @@ -0,0 +1,5 @@ +--- +title: Fixes a spelling error on the new label page +merge_request: 19316 +author: samdbeckham +type: fixed diff --git a/changelogs/unreleased/per-project-pipeline-iid.yml b/changelogs/unreleased/per-project-pipeline-iid.yml new file mode 100644 index 00000000000..78a513a9986 --- /dev/null +++ b/changelogs/unreleased/per-project-pipeline-iid.yml @@ -0,0 +1,5 @@ +--- +title: Add per-project pipeline id +merge_request: 18558 +author: +type: added diff --git a/changelogs/unreleased/reactive-caching-alive-bug.yml b/changelogs/unreleased/reactive-caching-alive-bug.yml new file mode 100644 index 00000000000..2fdc3a7e7e1 --- /dev/null +++ b/changelogs/unreleased/reactive-caching-alive-bug.yml @@ -0,0 +1,6 @@ +--- +title: Updates ReactiveCaching clear_reactive_caching method to clear both data and + alive caching +merge_request: 19311 +author: +type: fixed diff --git a/changelogs/unreleased/sh-bump-omniauth-gitlab.yml b/changelogs/unreleased/sh-bump-omniauth-gitlab.yml new file mode 100644 index 00000000000..145fdf72020 --- /dev/null +++ b/changelogs/unreleased/sh-bump-omniauth-gitlab.yml @@ -0,0 +1,5 @@ +--- +title: Bump omniauth-gitlab to 1.0.3 +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/sh-fix-secrets-not-working.yml b/changelogs/unreleased/sh-fix-secrets-not-working.yml new file mode 100644 index 00000000000..044a873ecd9 --- /dev/null +++ b/changelogs/unreleased/sh-fix-secrets-not-working.yml @@ -0,0 +1,5 @@ +--- +title: Fix attr_encryption key settings +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/sh-fix-source-project-nplus-one.yml b/changelogs/unreleased/sh-fix-source-project-nplus-one.yml new file mode 100644 index 00000000000..9d78ad6408c --- /dev/null +++ b/changelogs/unreleased/sh-fix-source-project-nplus-one.yml @@ -0,0 +1,5 @@ +--- +title: Fix N+1 with source_projects in merge requests API +merge_request: +author: +type: performance diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index dd36700964a..a0e3ab0d343 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -391,8 +391,10 @@ repositories_storages = Settings.repositories.storages.values repository_downloads_path = Settings.gitlab['repository_downloads_path'].to_s.gsub(%r{/$}, '') repository_downloads_full_path = File.expand_path(repository_downloads_path, Settings.gitlab['user_home']) -if repository_downloads_path.blank? || repositories_storages.any? { |rs| [repository_downloads_path, repository_downloads_full_path].include?(rs.legacy_disk_path.gsub(%r{/$}, '')) } - Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive') +Gitlab::GitalyClient::StorageSettings.allow_disk_access do + if repository_downloads_path.blank? || repositories_storages.any? { |rs| [repository_downloads_path, repository_downloads_full_path].include?(rs.legacy_disk_path.gsub(%r{/$}, '')) } + Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive') + end end # diff --git a/config/initializers/2_gitlab.rb b/config/initializers/2_gitlab.rb index 1d2ab606a63..8b7f245b7b0 100644 --- a/config/initializers/2_gitlab.rb +++ b/config/initializers/2_gitlab.rb @@ -1 +1 @@ -require_relative '../../lib/gitlab' +require_dependency 'gitlab' diff --git a/config/initializers/6_validations.rb b/config/initializers/6_validations.rb index 89aabe530fe..362a23164ab 100644 --- a/config/initializers/6_validations.rb +++ b/config/initializers/6_validations.rb @@ -38,10 +38,12 @@ def validate_storages_config end def validate_storages_paths - Gitlab.config.repositories.storages.each do |name, repository_storage| - parent_name, _parent_path = find_parent_path(name, repository_storage.legacy_disk_path) - if parent_name - storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages") + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.each do |name, repository_storage| + parent_name, _parent_path = find_parent_path(name, repository_storage.legacy_disk_path) + if parent_name + storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages") + end end end end diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index e33ebb25c4c..a7fa926a853 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -19,12 +19,5 @@ end if Gitlab.config.omniauth.enabled provider_names = Gitlab.config.omniauth.providers.map(&:name) - require 'omniauth-kerberos' if provider_names.include?('kerberos') -end - -module OmniAuth - module Strategies - autoload :Bitbucket, Rails.root.join('lib', 'omni_auth', 'strategies', 'bitbucket') - autoload :Jwt, Rails.root.join('lib', 'omni_auth', 'strategies', 'jwt') - end + Gitlab::Auth.omniauth_setup_providers(provider_names) end diff --git a/config/settings.rb b/config/settings.rb index 58f38d103ea..3f3481bb65d 100644 --- a/config/settings.rb +++ b/config/settings.rb @@ -85,17 +85,24 @@ class Settings < Settingslogic File.expand_path(path, Rails.root) end - # Returns a 256-bit key for attr_encrypted - def attr_encrypted_db_key_base - # Ruby 2.4+ requires passing in the exact required length for OpenSSL keys - # (https://github.com/ruby/ruby/commit/ce635262f53b760284d56bb1027baebaaec175d1). - # Previous versions quietly truncated the input. - # - # The default mode for the attr_encrypted gem is to use a 256-bit key. - # We truncate the 128-byte string to 32 bytes. + # Ruby 2.4+ requires passing in the exact required length for OpenSSL keys + # (https://github.com/ruby/ruby/commit/ce635262f53b760284d56bb1027baebaaec175d1). + # Previous versions quietly truncated the input. + # + # Use this when using :per_attribute_iv mode for attr_encrypted. + # We have to truncate the string to 32 bytes for a 256-bit cipher. + def attr_encrypted_db_key_base_truncated Gitlab::Application.secrets.db_key_base[0..31] end + # This should be used for :per_attribute_salt_and_iv mode. There is no + # need to truncate the key because the encryptor will use the salt to + # generate a hash of the password: + # https://github.com/attr-encrypted/encryptor/blob/c3a62c4a9e74686dd95e0548f9dc2a361fdc95d1/lib/encryptor.rb#L77 + def attr_encrypted_db_key_base + Gitlab::Application.secrets.db_key_base + end + private def base_url(config) diff --git a/db/migrate/20180424160449_add_pipeline_iid_to_ci_pipelines.rb b/db/migrate/20180424160449_add_pipeline_iid_to_ci_pipelines.rb new file mode 100644 index 00000000000..e8f0c91d612 --- /dev/null +++ b/db/migrate/20180424160449_add_pipeline_iid_to_ci_pipelines.rb @@ -0,0 +1,13 @@ +class AddPipelineIidToCiPipelines < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + add_column :ci_pipelines, :iid, :integer + end + + def down + remove_column :ci_pipelines, :iid, :integer + end +end diff --git a/db/migrate/20180425205249_add_index_constraints_to_pipeline_iid.rb b/db/migrate/20180425205249_add_index_constraints_to_pipeline_iid.rb new file mode 100644 index 00000000000..3fa59b44d5d --- /dev/null +++ b/db/migrate/20180425205249_add_index_constraints_to_pipeline_iid.rb @@ -0,0 +1,15 @@ +class AddIndexConstraintsToPipelineIid < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_pipelines, [:project_id, :iid], unique: true, where: 'iid IS NOT NULL' + end + + def down + remove_concurrent_index :ci_pipelines, [:project_id, :iid] + end +end diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 1586a7eb92f..a957f107405 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -48,7 +48,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati attr_encrypted :token, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base, + key: Settings.attr_encrypted_db_key_base_truncated, algorithm: 'aes-256-cbc' end diff --git a/db/schema.rb b/db/schema.rb index 97247387bc7..0d6b44d1b92 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -451,10 +451,12 @@ ActiveRecord::Schema.define(version: 20180529093006) do t.integer "config_source" t.boolean "protected" t.integer "failure_reason" + t.integer "iid" end add_index "ci_pipelines", ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree add_index "ci_pipelines", ["pipeline_schedule_id"], name: "index_ci_pipelines_on_pipeline_schedule_id", using: :btree + add_index "ci_pipelines", ["project_id", "iid"], name: "index_ci_pipelines_on_project_id_and_iid", unique: true, where: "(iid IS NOT NULL)", using: :btree add_index "ci_pipelines", ["project_id", "ref", "status", "id"], name: "index_ci_pipelines_on_project_id_and_ref_and_status_and_id", using: :btree add_index "ci_pipelines", ["project_id", "sha"], name: "index_ci_pipelines_on_project_id_and_sha", using: :btree add_index "ci_pipelines", ["project_id"], name: "index_ci_pipelines_on_project_id", using: :btree diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md index c0221533f13..9b1297ca4ba 100644 --- a/doc/administration/pages/index.md +++ b/doc/administration/pages/index.md @@ -83,12 +83,12 @@ you need to add a [wildcard DNS A record][wiki-wildcard-dns] pointing to the host that GitLab runs. For example, an entry would look like this: ``` -*.example.io. 1800 IN A 1.1.1.1 +*.example.io. 1800 IN A 192.0.2.1 *.example.io. 1800 IN AAAA 2001::1 ``` where `example.io` is the domain under which GitLab Pages will be served -and `1.1.1.1` is the IPv4 address of your GitLab instance and `2001::1` is the +and `192.0.2.1` is the IPv4 address of your GitLab instance and `2001::1` is the IPv6 address. If you don't have IPv6, you can omit the AAAA record. > **Note:** @@ -193,13 +193,13 @@ world. Custom domains are supported, but no TLS. ```shell pages_external_url "http://example.io" - nginx['listen_addresses'] = ['1.1.1.1'] + nginx['listen_addresses'] = ['192.0.2.1'] pages_nginx['enable'] = false - gitlab_pages['external_http'] = ['1.1.1.2:80', '[2001::2]:80'] + gitlab_pages['external_http'] = ['192.0.2.2:80', '[2001::2]:80'] ``` - where `1.1.1.1` is the primary IP address that GitLab is listening to and - `1.1.1.2` and `2001::2` are the secondary IPs the GitLab Pages daemon + where `192.0.2.1` is the primary IP address that GitLab is listening to and + `192.0.2.2` and `2001::2` are the secondary IPs the GitLab Pages daemon listens on. If you don't have IPv6, you can omit the IPv6 address. 1. [Reconfigure GitLab][reconfigure] @@ -228,16 +228,16 @@ world. Custom domains and TLS are supported. ```shell pages_external_url "https://example.io" - nginx['listen_addresses'] = ['1.1.1.1'] + nginx['listen_addresses'] = ['192.0.2.1'] pages_nginx['enable'] = false gitlab_pages['cert'] = "/etc/gitlab/ssl/example.io.crt" gitlab_pages['cert_key'] = "/etc/gitlab/ssl/example.io.key" - gitlab_pages['external_http'] = ['1.1.1.2:80', '[2001::2]:80'] - gitlab_pages['external_https'] = ['1.1.1.2:443', '[2001::2]:443'] + gitlab_pages['external_http'] = ['192.0.2.2:80', '[2001::2]:80'] + gitlab_pages['external_https'] = ['192.0.2.2:443', '[2001::2]:443'] ``` - where `1.1.1.1` is the primary IP address that GitLab is listening to and - `1.1.1.2` and `2001::2` are the secondary IPs where the GitLab Pages daemon + where `192.0.2.1` is the primary IP address that GitLab is listening to and + `192.0.2.2` and `2001::2` are the secondary IPs where the GitLab Pages daemon listens on. If you don't have IPv6, you can omit the IPv6 address. 1. [Reconfigure GitLab][reconfigure] diff --git a/doc/administration/pages/source.md b/doc/administration/pages/source.md index a45c3306457..4e40a7cb18d 100644 --- a/doc/administration/pages/source.md +++ b/doc/administration/pages/source.md @@ -67,11 +67,11 @@ you need to add a [wildcard DNS A record][wiki-wildcard-dns] pointing to the host that GitLab runs. For example, an entry would look like this: ``` -*.example.io. 1800 IN A 1.1.1.1 +*.example.io. 1800 IN A 192.0.2.1 ``` where `example.io` is the domain under which GitLab Pages will be served -and `1.1.1.1` is the IP address of your GitLab instance. +and `192.0.2.1` is the IP address of your GitLab instance. > **Note:** You should not use the GitLab domain to serve user pages. For more information @@ -253,7 +253,7 @@ world. Custom domains are supported, but no TLS. port: 80 https: false - external_http: 1.1.1.2:80 + external_http: 192.0.2.2:80 ``` 1. Edit `/etc/default/gitlab` and set `gitlab_pages_enabled` to `true` in @@ -263,7 +263,7 @@ world. Custom domains are supported, but no TLS. ``` gitlab_pages_enabled=true - gitlab_pages_options="-pages-domain example.io -pages-root $app_root/shared/pages -listen-proxy 127.0.0.1:8090 -listen-http 1.1.1.2:80" + gitlab_pages_options="-pages-domain example.io -pages-root $app_root/shared/pages -listen-proxy 127.0.0.1:8090 -listen-http 192.0.2.2:80" ``` 1. Copy the `gitlab-pages-ssl` Nginx configuration file: @@ -274,7 +274,7 @@ world. Custom domains are supported, but no TLS. ``` 1. Edit all GitLab related configs in `/etc/nginx/site-available/` and replace - `0.0.0.0` with `1.1.1.1`, where `1.1.1.1` the primary IP where GitLab + `0.0.0.0` with `192.0.2.1`, where `192.0.2.1` the primary IP where GitLab listens to. 1. Restart NGINX 1. [Restart GitLab][restart] @@ -320,8 +320,8 @@ world. Custom domains and TLS are supported. port: 443 https: true - external_http: 1.1.1.2:80 - external_https: 1.1.1.2:443 + external_http: 192.0.2.2:80 + external_https: 192.0.2.2:443 ``` 1. Edit `/etc/default/gitlab` and set `gitlab_pages_enabled` to `true` in @@ -333,7 +333,7 @@ world. Custom domains and TLS are supported. ``` gitlab_pages_enabled=true - gitlab_pages_options="-pages-domain example.io -pages-root $app_root/shared/pages -listen-proxy 127.0.0.1:8090 -listen-http 1.1.1.2:80 -listen-https 1.1.1.2:443 -root-cert /path/to/example.io.crt -root-key /path/to/example.io.key + gitlab_pages_options="-pages-domain example.io -pages-root $app_root/shared/pages -listen-proxy 127.0.0.1:8090 -listen-http 192.0.2.2:80 -listen-https 192.0.2.2:443 -root-cert /path/to/example.io.crt -root-key /path/to/example.io.key ``` 1. Copy the `gitlab-pages-ssl` Nginx configuration file: @@ -344,7 +344,7 @@ world. Custom domains and TLS are supported. ``` 1. Edit all GitLab related configs in `/etc/nginx/site-available/` and replace - `0.0.0.0` with `1.1.1.1`, where `1.1.1.1` the primary IP where GitLab + `0.0.0.0` with `192.0.2.1`, where `192.0.2.1` the primary IP where GitLab listens to. 1. Restart NGINX 1. [Restart GitLab][restart] diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 051d2a10bc6..62f9884e264 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -582,6 +582,7 @@ Parameters: "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, + "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, "time_stats": { diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index f10423b92cf..aa4395b01a9 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -72,6 +72,7 @@ future GitLab releases.** | **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 | @@ -352,6 +353,8 @@ Running on runner-8a2f473d-project-1796893-concurrent-0 via runner-8a2f473d-mach ++ CI_PROJECT_URL=https://example.com/gitlab-examples/ci-debug-trace ++ export CI_PIPELINE_ID=52666 ++ CI_PIPELINE_ID=52666 +++ export CI_PIPELINE_IID=123 +++ CI_PIPELINE_IID=123 ++ export CI_RUNNER_ID=1337 ++ CI_RUNNER_ID=1337 ++ export CI_RUNNER_DESCRIPTION=shared-runners-manager-1.example.com @@ -439,6 +442,7 @@ export CI_JOB_MANUAL="true" export CI_JOB_TRIGGERED="true" export CI_JOB_TOKEN="abcde-1234ABCD5678ef" export CI_PIPELINE_ID="1000" +export CI_PIPELINE_IID="10" export CI_PROJECT_ID="34" export CI_PROJECT_DIR="/builds/gitlab-org/gitlab-ce" export CI_PROJECT_NAME="gitlab-ce" diff --git a/doc/development/new_fe_guide/dependencies.md b/doc/development/new_fe_guide/dependencies.md index 3417d77a06d..12a4f089d41 100644 --- a/doc/development/new_fe_guide/dependencies.md +++ b/doc/development/new_fe_guide/dependencies.md @@ -1,3 +1,20 @@ # Dependencies -> TODO: Add Dependencies
\ No newline at end of file +## Adding Dependencies. + +GitLab uses `yarn` to manage dependencies. These dependencies are defined in +two groups within `package.json`, `dependencies` and `devDependencies`. For +our purposes, we consider anything that is required to compile our production +assets a "production" dependency. That is, anything required to run the +`webpack` script with `NODE_ENV=production`. Tools like `eslint`, `karma`, and +various plugins and tools used in development are considered `devDependencies`. +This distinction is used by omnibus to determine which dependencies it requires +when building GitLab. + +Exceptions are made for some tools that we require in the +`gitlab:assets:compile` CI job such as `webpack-bundle-analyzer` to analyze our +production assets post-compile. + +--- + +> TODO: Add Dependencies diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index 98af87455ec..e1d1969651e 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -144,7 +144,7 @@ helm install --name gitlab -f values.yaml gitlab/gitlab-omnibus or passing them on the command line: ```bash -helm install --name gitlab --set baseDomain=gitlab.io,baseIP=1.1.1.1,gitlab=ee,gitlabEELicense=$LICENSE,legoEmail=email@gitlab.com gitlab/gitlab-omnibus +helm install --name gitlab --set baseDomain=gitlab.io,baseIP=192.0.2.1,gitlab=ee,gitlabEELicense=$LICENSE,legoEmail=email@gitlab.com gitlab/gitlab-omnibus ``` ## Updating GitLab using the Helm Chart diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index b1e510d72de..278d53427f0 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -38,7 +38,7 @@ module API merge_requests = MergeRequestsFinder.new(current_user, args).execute .reorder(args[:order_by] => args[:sort]) merge_requests = paginate(merge_requests) - .preload(:target_project) + .preload(:source_project, :target_project) return merge_requests if args[:view] == 'simple' diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 5b7ae89440c..e9886c76870 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -21,24 +21,26 @@ module API attributes = attributes_for_keys([:description, :active, :locked, :run_untagged, :tag_list, :maximum_timeout]) .merge(get_runner_details_from_request) - runner = + attributes = if runner_registration_token_valid? # Create shared runner. Requires admin access - Ci::Runner.create(attributes.merge(is_shared: true, runner_type: :instance_type)) + attributes.merge(is_shared: true, runner_type: :instance_type) elsif project = Project.find_by(runners_token: params[:token]) # Create a specific runner for the project - project.runners.create(attributes.merge(runner_type: :project_type)) + attributes.merge(is_shared: false, runner_type: :project_type, projects: [project]) elsif group = Group.find_by(runners_token: params[:token]) # Create a specific runner for the group - group.runners.create(attributes.merge(runner_type: :group_type)) + attributes.merge(is_shared: false, runner_type: :group_type, groups: [group]) + else + forbidden! end - break forbidden! unless runner + runner = Ci::Runner.create(attributes) - if runner.id + if runner.persisted? present runner, with: Entities::RunnerRegistrationDetails else - not_found! + render_validation_error!(runner) end end diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 5cb96d467c0..2b78075ddbf 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -133,12 +133,10 @@ module API runner = get_runner(params[:runner_id]) authenticate_enable_runner!(runner) - runner_project = runner.assign_to(user_project) - - if runner_project.persisted? + if runner.assign_to(user_project) present runner, with: Entities::Runner else - conflict!("Runner was already enabled for this project") + render_validation_error!(runner) end end diff --git a/lib/api/search.rb b/lib/api/search.rb index 5d9ec617cb7..37fbabe419c 100644 --- a/lib/api/search.rb +++ b/lib/api/search.rb @@ -34,9 +34,7 @@ module API def process_results(results) case params[:scope] - when 'wiki_blobs' - paginate(results).map { |blob| Gitlab::ProjectSearchResults.parse_search_result(blob, user_project) } - when 'blobs' + when 'blobs', 'wiki_blobs' paginate(results).map { |blob| blob[1] } else paginate(results) diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 8e5a985edd7..0f7a7b0ce8d 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -14,6 +14,25 @@ module Gitlab DEFAULT_SCOPES = [:api].freeze class << self + def omniauth_customized_providers + @omniauth_customized_providers ||= %w[bitbucket jwt] + end + + def omniauth_setup_providers(provider_names) + provider_names.each do |provider| + omniauth_setup_a_provider(provider) + end + end + + def omniauth_setup_a_provider(provider) + case provider + when 'kerberos' + require 'omniauth-kerberos' + when *omniauth_customized_providers + require_dependency "omni_auth/strategies/#{provider}" + end + end + def find_for_git_client(login, password, project:, ip:) raise "Must provide an IP for rate limiting" if ip.nil? diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb index 69b8a8fc68f..f34c11ca3c2 100644 --- a/lib/gitlab/ci/pipeline/chain/populate.rb +++ b/lib/gitlab/ci/pipeline/chain/populate.rb @@ -8,6 +8,9 @@ module Gitlab PopulateError = Class.new(StandardError) def perform! + # Allocate next IID. This operation must be outside of transactions of pipeline creations. + pipeline.ensure_project_iid! + ## # Populate pipeline with block argument of CreatePipelineService#execute. # diff --git a/lib/gitlab/ci/trace/http_io.rb b/lib/gitlab/ci/trace/http_io.rb index cff924e27ef..8788af57a67 100644 --- a/lib/gitlab/ci/trace/http_io.rb +++ b/lib/gitlab/ci/trace/http_io.rb @@ -148,7 +148,7 @@ module Gitlab def get_chunk unless in_range? - response = Net::HTTP.start(uri.hostname, uri.port, use_ssl: uri.scheme == 'https') do |http| + response = Net::HTTP.start(uri.hostname, uri.port, proxy_from_env: true, use_ssl: uri.scheme == 'https') do |http| http.request(request) end diff --git a/lib/gitlab/cycle_analytics/summary/commit.rb b/lib/gitlab/cycle_analytics/summary/commit.rb index bea78862757..0a88e052f60 100644 --- a/lib/gitlab/cycle_analytics/summary/commit.rb +++ b/lib/gitlab/cycle_analytics/summary/commit.rb @@ -7,7 +7,9 @@ module Gitlab end def value - @value ||= count_commits + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + @value ||= count_commits + end end private diff --git a/lib/gitlab/file_finder.rb b/lib/gitlab/file_finder.rb index 8c082c0c336..f42088f980e 100644 --- a/lib/gitlab/file_finder.rb +++ b/lib/gitlab/file_finder.rb @@ -32,17 +32,13 @@ module Gitlab end def find_by_filename(query, except: []) - filenames = repository.search_files_by_name(query, ref).first(BATCH_SIZE) - filenames.delete_if { |filename| except.include?(filename) } unless except.empty? + filenames = search_filenames(query, except) - blob_refs = filenames.map { |filename| [ref, filename] } - blobs = Gitlab::Git::Blob.batch(repository, blob_refs, blob_size_limit: 1024) - - blobs.map do |blob| + blobs(filenames).map do |blob| Gitlab::SearchResults::FoundBlob.new( id: blob.id, filename: blob.path, - basename: File.basename(blob.path), + basename: File.basename(blob.path, File.extname(blob.path)), ref: ref, startline: 1, data: blob.data, @@ -50,5 +46,21 @@ module Gitlab ) end end + + def search_filenames(query, except) + filenames = repository.search_files_by_name(query, ref).first(BATCH_SIZE) + + filenames.delete_if { |filename| except.include?(filename) } unless except.empty? + + filenames + end + + def blob_refs(filenames) + filenames.map { |filename| [ref, filename] } + end + + def blobs(filenames) + Gitlab::Git::Blob.batch(repository, blob_refs(filenames), blob_size_limit: 1024) + end end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 1a21625a322..4cbf20bfe76 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1185,15 +1185,17 @@ module Gitlab end def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:) - with_repo_branch_commit(source_repository, source_branch_name) do |commit| - break unless commit - - Gitlab::Git::Compare.new( - self, - target_branch_name, - commit.sha, - straight: straight - ) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + with_repo_branch_commit(source_repository, source_branch_name) do |commit| + break unless commit + + Gitlab::Git::Compare.new( + self, + target_branch_name, + commit.sha, + straight: straight + ) + end end end @@ -1455,7 +1457,7 @@ module Gitlab gitaly_repository_client.cleanup if is_enabled && exists? end rescue Gitlab::Git::CommandError => e # Don't fail if we can't cleanup - Rails.logger.error("Unable to clean repository on storage #{storage} with path #{path}: #{e.message}") + Rails.logger.error("Unable to clean repository on storage #{storage} with relative path #{relative_path}: #{e.message}") Gitlab::Metrics.counter( :failed_repository_cleanup_total, 'Number of failed repository cleanup events' diff --git a/lib/gitlab/git/storage/checker.rb b/lib/gitlab/git/storage/checker.rb index 2f611cef37b..391f0d70583 100644 --- a/lib/gitlab/git/storage/checker.rb +++ b/lib/gitlab/git/storage/checker.rb @@ -35,7 +35,7 @@ module Gitlab def initialize(storage, logger = Rails.logger) @storage = storage config = Gitlab.config.repositories.storages[@storage] - @storage_path = config.legacy_disk_path + @storage_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { config.legacy_disk_path } @logger = logger @hostname = Gitlab::Environment.hostname diff --git a/lib/gitlab/git/storage/circuit_breaker.rb b/lib/gitlab/git/storage/circuit_breaker.rb index e35054466ff..62427ac9cc4 100644 --- a/lib/gitlab/git/storage/circuit_breaker.rb +++ b/lib/gitlab/git/storage/circuit_breaker.rb @@ -22,13 +22,14 @@ module Gitlab def self.build(storage, hostname = Gitlab::Environment.hostname) config = Gitlab.config.repositories.storages[storage] - - if !config.present? - NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Storage '#{storage}' is not configured")) - elsif !config.legacy_disk_path.present? - NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Path for storage '#{storage}' is not configured")) - else - new(storage, hostname) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + if !config.present? + NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Storage '#{storage}' is not configured")) + elsif !config.legacy_disk_path.present? + NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Path for storage '#{storage}' is not configured")) + else + new(storage, hostname) + end end end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 0abae70c443..550294916a4 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -33,6 +33,11 @@ module Gitlab MAXIMUM_GITALY_CALLS = 35 CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze + # We have a mechanism to let GitLab automatically opt in to all Gitaly + # features. We want to be able to exclude some features from automatic + # opt-in. That is what EXPLICIT_OPT_IN_REQUIRED is for. + EXPLICIT_OPT_IN_REQUIRED = [Gitlab::GitalyClient::StorageSettings::DISK_ACCESS_DENIED_FLAG].freeze + MUTEX = Mutex.new class << self @@ -234,7 +239,7 @@ module Gitlab when MigrationStatus::OPT_OUT true when MigrationStatus::OPT_IN - opt_into_all_features? + opt_into_all_features? && !EXPLICIT_OPT_IN_REQUIRED.include?(feature_name) else false end diff --git a/lib/gitlab/gitaly_client/storage_settings.rb b/lib/gitlab/gitaly_client/storage_settings.rb index 9a576e463e3..02fcb413abd 100644 --- a/lib/gitlab/gitaly_client/storage_settings.rb +++ b/lib/gitlab/gitaly_client/storage_settings.rb @@ -4,6 +4,8 @@ module Gitlab # where production code (app, config, db, lib) touches Git repositories # directly. class StorageSettings + extend Gitlab::TemporarilyAllow + DirectPathAccessError = Class.new(StandardError) InvalidConfigurationError = Class.new(StandardError) @@ -17,7 +19,21 @@ module Gitlab # This class will give easily recognizable NoMethodErrors Deprecated = Class.new - attr_reader :legacy_disk_path + MUTEX = Mutex.new + + DISK_ACCESS_DENIED_FLAG = :deny_disk_access + ALLOW_KEY = :allow_disk_access + + # If your code needs this method then your code needs to be fixed. + def self.allow_disk_access + temporarily_allow(ALLOW_KEY) { yield } + end + + def self.disk_access_denied? + !temporarily_allowed?(ALLOW_KEY) && GitalyClient.feature_enabled?(DISK_ACCESS_DENIED_FLAG) + rescue + false # Err on the side of caution, don't break gitlab for people + end def initialize(storage) raise InvalidConfigurationError, "expected a Hash, got a #{storage.class.name}" unless storage.is_a?(Hash) @@ -34,6 +50,14 @@ module Gitlab @hash.fetch(:gitaly_address) end + def legacy_disk_path + if self.class.disk_access_denied? + raise DirectPathAccessError, "git disk access denied via the gitaly_#{DISK_ACCESS_DENIED_FLAG} feature" + end + + @legacy_disk_path + end + private def method_missing(m, *args, &block) diff --git a/lib/gitlab/health_checks/fs_shards_check.rb b/lib/gitlab/health_checks/fs_shards_check.rb index 6e554383270..fcbf266b80b 100644 --- a/lib/gitlab/health_checks/fs_shards_check.rb +++ b/lib/gitlab/health_checks/fs_shards_check.rb @@ -77,7 +77,9 @@ module Gitlab end def storage_path(storage_name) - storages_paths[storage_name]&.legacy_disk_path + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + storages_paths[storage_name]&.legacy_disk_path + end end # All below test methods use shell commands to perform actions on storage volumes. diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 2e9b6e302f5..38bdc61d8ab 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -106,7 +106,8 @@ module Gitlab project_wiki = ProjectWiki.new(project) unless project_wiki.empty? - project_wiki.search_files(query) + ref = repository_ref || project.wiki.default_branch + Gitlab::WikiFileFinder.new(project, ref).find(query) else [] end diff --git a/lib/gitlab/temporarily_allow.rb b/lib/gitlab/temporarily_allow.rb new file mode 100644 index 00000000000..880e55f71df --- /dev/null +++ b/lib/gitlab/temporarily_allow.rb @@ -0,0 +1,42 @@ +module Gitlab + module TemporarilyAllow + TEMPORARILY_ALLOW_MUTEX = Mutex.new + + def temporarily_allow(key) + temporarily_allow_add(key, 1) + yield + ensure + temporarily_allow_add(key, -1) + end + + def temporarily_allowed?(key) + if RequestStore.active? + temporarily_allow_request_store[key] > 0 + else + TEMPORARILY_ALLOW_MUTEX.synchronize do + temporarily_allow_ivar[key] > 0 + end + end + end + + private + + def temporarily_allow_ivar + @temporarily_allow ||= Hash.new(0) + end + + def temporarily_allow_request_store + RequestStore[:temporarily_allow] ||= Hash.new(0) + end + + def temporarily_allow_add(key, value) + if RequestStore.active? + temporarily_allow_request_store[key] += value + else + TEMPORARILY_ALLOW_MUTEX.synchronize do + temporarily_allow_ivar[key] += value + end + end + end + end +end diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index db97f65bd54..20be193ea0c 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -5,7 +5,7 @@ module Gitlab BlockedUrlError = Class.new(StandardError) class << self - def validate!(url, allow_localhost: false, allow_local_network: true, valid_ports: []) + def validate!(url, allow_localhost: false, allow_local_network: true, ports: [], protocols: []) return true if url.nil? begin @@ -18,7 +18,8 @@ module Gitlab return true if internal?(uri) port = uri.port || uri.default_port - validate_port!(port, valid_ports) if valid_ports.any? + validate_protocol!(uri.scheme, protocols) + validate_port!(port, ports) if ports.any? validate_user!(uri.user) validate_hostname!(uri.hostname) @@ -44,13 +45,19 @@ module Gitlab private - def validate_port!(port, valid_ports) + def validate_port!(port, ports) return if port.blank? # Only ports under 1024 are restricted return if port >= 1024 - return if valid_ports.include?(port) + return if ports.include?(port) - raise BlockedUrlError, "Only allowed ports are #{valid_ports.join(', ')}, and any over 1024" + raise BlockedUrlError, "Only allowed ports are #{ports.join(', ')}, and any over 1024" + end + + def validate_protocol!(protocol, protocols) + if protocol.blank? || (protocols.any? && !protocols.include?(protocol)) + raise BlockedUrlError, "Only allowed protocols are #{protocols.join(', ')}" + end end def validate_user!(value) diff --git a/lib/gitlab/wiki_file_finder.rb b/lib/gitlab/wiki_file_finder.rb new file mode 100644 index 00000000000..f97278f05cd --- /dev/null +++ b/lib/gitlab/wiki_file_finder.rb @@ -0,0 +1,23 @@ +module Gitlab + class WikiFileFinder < FileFinder + attr_reader :repository + + def initialize(project, ref) + @project = project + @ref = ref + @repository = project.wiki.repository + end + + private + + def search_filenames(query, except) + safe_query = Regexp.escape(query.tr(' ', '-')) + safe_query = Regexp.new(safe_query, Regexp::IGNORECASE) + filenames = repository.ls_files(ref) + + filenames.delete_if { |filename| except.include?(filename) } unless except.empty? + + filenames.grep(safe_query).first(BATCH_SIZE) + end + end +end diff --git a/lib/omni_auth/strategies/jwt.rb b/lib/omni_auth/strategies/jwt.rb index 2349b2a28aa..ebdb5c7faf0 100644 --- a/lib/omni_auth/strategies/jwt.rb +++ b/lib/omni_auth/strategies/jwt.rb @@ -3,7 +3,7 @@ require 'jwt' module OmniAuth module Strategies - class JWT + class Jwt ClaimInvalid = Class.new(StandardError) include OmniAuth::Strategy @@ -56,7 +56,5 @@ module OmniAuth fail! :claim_invalid, e end end - - class Jwt < JWT; end end end diff --git a/lib/rspec_flaky/listener.rb b/lib/rspec_flaky/listener.rb index 5b5e4f7c7de..9cd0c38cb55 100644 --- a/lib/rspec_flaky/listener.rb +++ b/lib/rspec_flaky/listener.rb @@ -1,10 +1,10 @@ require 'json' -require_relative 'config' -require_relative 'example' -require_relative 'flaky_example' -require_relative 'flaky_examples_collection' -require_relative 'report' +require_dependency 'rspec_flaky/config' +require_dependency 'rspec_flaky/example' +require_dependency 'rspec_flaky/flaky_example' +require_dependency 'rspec_flaky/flaky_examples_collection' +require_dependency 'rspec_flaky/report' module RspecFlaky class Listener diff --git a/lib/rspec_flaky/report.rb b/lib/rspec_flaky/report.rb index a8730d3b7c7..1c362fdd20d 100644 --- a/lib/rspec_flaky/report.rb +++ b/lib/rspec_flaky/report.rb @@ -1,8 +1,8 @@ require 'json' require 'time' -require_relative 'config' -require_relative 'flaky_examples_collection' +require_dependency 'rspec_flaky/config' +require_dependency 'rspec_flaky/flaky_examples_collection' module RspecFlaky # This class is responsible for loading/saving JSON reports, and pruning diff --git a/lib/tasks/lint.rake b/lib/tasks/lint.rake index fe5032cae18..8b86a5c72a5 100644 --- a/lib/tasks/lint.rake +++ b/lib/tasks/lint.rake @@ -30,11 +30,12 @@ unless Rails.env.production? lint:static_verification ].each do |task| pid = Process.fork do - rd, wr = IO.pipe + rd_out, wr_out = IO.pipe + rd_err, wr_err = IO.pipe stdout = $stdout.dup stderr = $stderr.dup - $stdout.reopen(wr) - $stderr.reopen(wr) + $stdout.reopen(wr_out) + $stderr.reopen(wr_err) begin begin @@ -48,14 +49,13 @@ unless Rails.env.production? ensure $stdout.reopen(stdout) $stderr.reopen(stderr) - wr.close + wr_out.close + wr_err.close - if msg - warn "\n#{msg}\n\n" - IO.copy_stream(rd, $stderr) - else - IO.copy_stream(rd, $stdout) - end + warn "\n#{msg}\n\n" if msg + + IO.copy_stream(rd_out, $stdout) + IO.copy_stream(rd_err, $stderr) end end diff --git a/scripts/prune-old-flaky-specs b/scripts/prune-old-flaky-specs index f7451fbd428..a00a334fd6e 100755 --- a/scripts/prune-old-flaky-specs +++ b/scripts/prune-old-flaky-specs @@ -5,7 +5,11 @@ # gem manually on the CI require 'rubygems' -require_relative '../lib/rspec_flaky/report' +# In newer Ruby, alias_method is not private then we don't need __send__ +singleton_class.__send__(:alias_method, :require_dependency, :require) # rubocop:disable GitlabSecurity/PublicSend +$:.unshift(File.expand_path('../lib', __dir__)) + +require 'rspec_flaky/report' report_file = ARGV.shift unless report_file diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb index 6d31b0ce959..5770d15557c 100644 --- a/spec/controllers/groups/runners_controller_spec.rb +++ b/spec/controllers/groups/runners_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Groups::RunnersController do let(:user) { create(:user) } let(:group) { create(:group) } - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } let(:params) do { @@ -15,7 +15,6 @@ describe Groups::RunnersController do before do sign_in(user) group.add_master(user) - group.runners << runner end describe '#update' do diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index 45c1218a39c..5d64f362252 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -54,7 +54,7 @@ describe Projects::MirrorsController do do_put(project, remote_mirrors_attributes: remote_mirror_attributes) expect(response).to redirect_to(project_settings_repository_path(project)) - expect(flash[:alert]).to match(/must be a valid URL/) + expect(flash[:alert]).to match(/Only allowed protocols are/) end it 'should not create a RemoteMirror object' do diff --git a/spec/controllers/projects/runners_controller_spec.rb b/spec/controllers/projects/runners_controller_spec.rb index 89a13f3c976..2082dd2cff0 100644 --- a/spec/controllers/projects/runners_controller_spec.rb +++ b/spec/controllers/projects/runners_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Projects::RunnersController do let(:user) { create(:user) } let(:project) { create(:project) } - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:params) do { @@ -16,7 +16,6 @@ describe Projects::RunnersController do before do sign_in(user) project.add_master(user) - project.runners << runner end describe '#update' do diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index e4dc61b3a68..61f35cf325b 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -102,7 +102,7 @@ describe Projects::ServicesController do expect(response.status).to eq(200) expect(JSON.parse(response.body)) - .to eq('error' => true, 'message' => 'Test failed.', 'service_response' => 'Bad test') + .to eq('error' => true, 'message' => 'Test failed.', 'service_response' => 'Bad test', 'test_failed' => true) end end end diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index f1810763d2d..d53fe9bf734 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -19,12 +19,12 @@ describe Projects::Settings::CiCdController do end context 'with group runners' do - let(:group_runner) { create(:ci_runner, runner_type: :group_type) } let(:parent_group) { create(:group) } - let(:group) { create(:group, runners: [group_runner], parent: parent_group) } + let(:group) { create(:group, parent: parent_group) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } let(:other_project) { create(:project, group: group) } - let!(:project_runner) { create(:ci_runner, projects: [other_project], runner_type: :project_type) } - let!(:shared_runner) { create(:ci_runner, :shared) } + let!(:project_runner) { create(:ci_runner, :project, projects: [other_project]) } + let!(:shared_runner) { create(:ci_runner, :instance) } it 'sets assignable project runners only' do group.add_master(user) diff --git a/spec/factories/ci/runner_projects.rb b/spec/factories/ci/runner_projects.rb index f605e90ceed..ec15972c423 100644 --- a/spec/factories/ci/runner_projects.rb +++ b/spec/factories/ci/runner_projects.rb @@ -1,6 +1,6 @@ FactoryBot.define do factory :ci_runner_project, class: Ci::RunnerProject do - runner factory: :ci_runner + runner factory: [:ci_runner, :project] project end end diff --git a/spec/factories/ci/runners.rb b/spec/factories/ci/runners.rb index cdc170b9ccb..6fb621b5e51 100644 --- a/spec/factories/ci/runners.rb +++ b/spec/factories/ci/runners.rb @@ -3,22 +3,45 @@ FactoryBot.define do sequence(:description) { |n| "My runner#{n}" } platform "darwin" - is_shared false active true access_level :not_protected - runner_type :project_type + + is_shared true + runner_type :instance_type trait :online do contacted_at Time.now end - trait :shared do + trait :instance do is_shared true runner_type :instance_type end - trait :specific do + trait :group do + is_shared false + runner_type :group_type + + after(:build) do |runner, evaluator| + runner.groups << build(:group) if runner.groups.empty? + end + end + + trait :project do is_shared false + runner_type :project_type + + after(:build) do |runner, evaluator| + runner.projects << build(:project) if runner.projects.empty? + end + end + + trait :without_projects do + # we use that to create invalid runner: + # the one without projects + after(:create) do |runner, evaluator| + runner.runner_projects.delete_all + end end trait :inactive do diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index 998080a3dd5..3a35bdd25de 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -3,6 +3,7 @@ FactoryBot.define do title { generate(:title) } project author { project.creator } + updated_by { author } trait :confidential do confidential true diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index c33014cbb31..be8754a5315 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -62,7 +62,7 @@ describe "Admin Runners" do context 'group runner' do let(:group) { create(:group) } - let!(:runner) { create(:ci_runner, groups: [group], runner_type: :group_type) } + let!(:runner) { create(:ci_runner, :group, groups: [group]) } it 'shows the label and does not show the project count' do visit admin_runners_path @@ -76,7 +76,7 @@ describe "Admin Runners" do context 'shared runner' do it 'shows the label and does not show the project count' do - runner = create :ci_runner, :shared + runner = create :ci_runner, :instance visit admin_runners_path @@ -90,7 +90,7 @@ describe "Admin Runners" do context 'specific runner' do it 'shows the label and the project count' do project = create :project - runner = create :ci_runner, projects: [project] + runner = create :ci_runner, :project, projects: [project] visit admin_runners_path @@ -149,8 +149,9 @@ describe "Admin Runners" do end context 'with specific runner' do + let(:runner) { create(:ci_runner, :project, projects: [@project1]) } + before do - @project1.runners << runner visit admin_runner_path(runner) end @@ -158,9 +159,9 @@ describe "Admin Runners" do end context 'with locked runner' do + let(:runner) { create(:ci_runner, :project, projects: [@project1], locked: true) } + before do - runner.update(locked: true) - @project1.runners << runner visit admin_runner_path(runner) end @@ -168,9 +169,10 @@ describe "Admin Runners" do end context 'with shared runner' do + let(:runner) { create(:ci_runner, :instance) } + before do @project1.destroy - runner.update(is_shared: true) visit admin_runner_path(runner) end @@ -179,8 +181,9 @@ describe "Admin Runners" do end describe 'disable/destroy' do + let(:runner) { create(:ci_runner, :project, projects: [@project1]) } + before do - @project1.runners << runner visit admin_runner_path(runner) end diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index e0cd963fe39..9ce7d538004 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -28,12 +28,8 @@ feature 'Runners' do project.add_master(user) end - context 'when a specific runner is activated on the project' do - given(:specific_runner) { create(:ci_runner, :specific) } - - background do - project.runners << specific_runner - end + context 'when a project_type runner is activated on the project' do + given!(:specific_runner) { create(:ci_runner, :project, projects: [project]) } scenario 'user sees the specific runner' do visit project_runners_path(project) @@ -114,7 +110,7 @@ feature 'Runners' do end context 'when a shared runner is activated on the project' do - given!(:shared_runner) { create(:ci_runner, :shared) } + given!(:shared_runner) { create(:ci_runner, :instance) } scenario 'user sees CI/CD setting page' do visit project_runners_path(project) @@ -126,11 +122,10 @@ feature 'Runners' do context 'when a specific runner exists in another project' do given(:another_project) { create(:project) } - given(:specific_runner) { create(:ci_runner, :specific) } + given!(:specific_runner) { create(:ci_runner, :project, projects: [another_project]) } background do another_project.add_master(user) - another_project.runners << specific_runner end scenario 'user enables and disables a specific runner' do @@ -220,8 +215,8 @@ feature 'Runners' do end context 'project with a group but no group runner' do - given(:group) { create :group } - given(:project) { create :project, group: group } + given(:group) { create(:group) } + given(:project) { create(:project, group: group) } scenario 'group runners are not available' do visit project_runners_path(project) @@ -234,9 +229,9 @@ feature 'Runners' do end context 'project with a group and a group runner' do - given(:group) { create :group } - given(:project) { create :project, group: group } - given!(:ci_runner) { create :ci_runner, groups: [group], description: 'group-runner' } + given(:group) { create(:group) } + given(:project) { create(:project, group: group) } + given!(:ci_runner) { create(:ci_runner, :group, groups: [group], description: 'group-runner') } scenario 'group runners are available' do visit project_runners_path(project) @@ -263,7 +258,7 @@ feature 'Runners' do end context 'group runners in group settings' do - given(:group) { create :group } + given(:group) { create(:group) } background do group.add_master(user) end @@ -277,7 +272,7 @@ feature 'Runners' do end context 'group with a runner' do - let!(:runner) { create :ci_runner, groups: [group], description: 'group-runner' } + let!(:runner) { create(:ci_runner, :group, groups: [group], description: 'group-runner') } scenario 'the runner is visible' do visit group_settings_ci_cd_path(group) diff --git a/spec/finders/runner_jobs_finder_spec.rb b/spec/finders/runner_jobs_finder_spec.rb index 4275b1a7ff1..97304170c4e 100644 --- a/spec/finders/runner_jobs_finder_spec.rb +++ b/spec/finders/runner_jobs_finder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe RunnerJobsFinder do let(:project) { create(:project) } - let(:runner) { create(:ci_runner, :shared) } + let(:runner) { create(:ci_runner, :instance) } subject { described_class.new(runner, params).execute } diff --git a/spec/fixtures/api/schemas/entities/issue.json b/spec/fixtures/api/schemas/entities/issue.json index 38467b4ca20..00abe73ec8a 100644 --- a/spec/fixtures/api/schemas/entities/issue.json +++ b/spec/fixtures/api/schemas/entities/issue.json @@ -27,7 +27,7 @@ "due_date": { "type": "date" }, "confidential": { "type": "boolean" }, "discussion_locked": { "type": ["boolean", "null"] }, - "updated_by_id": { "type": ["string", "null"] }, + "updated_by_id": { "type": ["integer", "null"] }, "time_estimate": { "type": "integer" }, "total_time_spent": { "type": "integer" }, "human_time_estimate": { "type": ["integer", "null"] }, diff --git a/spec/javascripts/ide/helpers.js b/spec/javascripts/ide/helpers.js index 5c7e2db0e96..9312e17704e 100644 --- a/spec/javascripts/ide/helpers.js +++ b/spec/javascripts/ide/helpers.js @@ -1,12 +1,14 @@ import { decorateData } from '~/ide/stores/utils'; import state from '~/ide/stores/state'; import commitState from '~/ide/stores/modules/commit/state'; +import mergeRequestsState from '~/ide/stores/modules/merge_requests/state'; import pipelinesState from '~/ide/stores/modules/pipelines/state'; export const resetStore = store => { const newState = { ...state(), commit: commitState(), + mergeRequests: mergeRequestsState(), pipelines: pipelinesState(), }; store.replaceState(newState); diff --git a/spec/javascripts/ide/mock_data.js b/spec/javascripts/ide/mock_data.js index 8ad51e122b6..dcf857f7e04 100644 --- a/spec/javascripts/ide/mock_data.js +++ b/spec/javascripts/ide/mock_data.js @@ -147,3 +147,13 @@ export const fullPipelinesResponse = { ], }, }; + +export const mergeRequests = [ + { + id: 1, + iid: 1, + title: 'Test merge request', + project_id: 1, + web_url: `${gl.TEST_HOST}/namespace/project-path/merge_requests/1`, + }, +]; diff --git a/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js b/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js new file mode 100644 index 00000000000..b571cfb963a --- /dev/null +++ b/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js @@ -0,0 +1,182 @@ +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; +import state from '~/ide/stores/modules/merge_requests/state'; +import * as types from '~/ide/stores/modules/merge_requests/mutation_types'; +import actions, { + requestMergeRequests, + receiveMergeRequestsError, + receiveMergeRequestsSuccess, + fetchMergeRequests, + resetMergeRequests, +} from '~/ide/stores/modules/merge_requests/actions'; +import { mergeRequests } from '../../../mock_data'; +import testAction from '../../../../helpers/vuex_action_helper'; + +describe('IDE merge requests actions', () => { + let mockedState; + let mock; + + beforeEach(() => { + mockedState = state(); + mock = new MockAdapter(axios); + }); + + afterEach(() => { + mock.restore(); + }); + + describe('requestMergeRequests', () => { + it('should should commit request', done => { + testAction( + requestMergeRequests, + null, + mockedState, + [{ type: types.REQUEST_MERGE_REQUESTS }], + [], + done, + ); + }); + }); + + describe('receiveMergeRequestsError', () => { + let flashSpy; + + beforeEach(() => { + flashSpy = spyOnDependency(actions, 'flash'); + }); + + it('should should commit error', done => { + testAction( + receiveMergeRequestsError, + null, + mockedState, + [{ type: types.RECEIVE_MERGE_REQUESTS_ERROR }], + [], + done, + ); + }); + + it('creates flash message', () => { + receiveMergeRequestsError({ commit() {} }); + + expect(flashSpy).toHaveBeenCalled(); + }); + }); + + describe('receiveMergeRequestsSuccess', () => { + it('should commit received data', done => { + testAction( + receiveMergeRequestsSuccess, + 'data', + mockedState, + [{ type: types.RECEIVE_MERGE_REQUESTS_SUCCESS, payload: 'data' }], + [], + done, + ); + }); + }); + + describe('fetchMergeRequests', () => { + beforeEach(() => { + gon.api_version = 'v4'; + }); + + describe('success', () => { + beforeEach(() => { + mock.onGet(/\/api\/v4\/merge_requests(.*)$/).replyOnce(200, mergeRequests); + }); + + it('calls API with params from state', () => { + const apiSpy = spyOn(axios, 'get').and.callThrough(); + + fetchMergeRequests({ dispatch() {}, state: mockedState }); + + expect(apiSpy).toHaveBeenCalledWith(jasmine.anything(), { + params: { + scope: 'assigned-to-me', + state: 'opened', + search: '', + }, + }); + }); + + it('calls API with search', () => { + const apiSpy = spyOn(axios, 'get').and.callThrough(); + + fetchMergeRequests({ dispatch() {}, state: mockedState }, 'testing search'); + + expect(apiSpy).toHaveBeenCalledWith(jasmine.anything(), { + params: { + scope: 'assigned-to-me', + state: 'opened', + search: 'testing search', + }, + }); + }); + + it('dispatches request', done => { + testAction( + fetchMergeRequests, + null, + mockedState, + [], + [ + { type: 'requestMergeRequests' }, + { type: 'resetMergeRequests' }, + { type: 'receiveMergeRequestsSuccess' }, + ], + done, + ); + }); + + it('dispatches success with received data', done => { + testAction( + fetchMergeRequests, + null, + mockedState, + [], + [ + { type: 'requestMergeRequests' }, + { type: 'resetMergeRequests' }, + { type: 'receiveMergeRequestsSuccess', payload: mergeRequests }, + ], + done, + ); + }); + }); + + describe('error', () => { + beforeEach(() => { + mock.onGet(/\/api\/v4\/merge_requests(.*)$/).replyOnce(500); + }); + + it('dispatches error', done => { + testAction( + fetchMergeRequests, + null, + mockedState, + [], + [ + { type: 'requestMergeRequests' }, + { type: 'resetMergeRequests' }, + { type: 'receiveMergeRequestsError' }, + ], + done, + ); + }); + }); + }); + + describe('resetMergeRequests', () => { + it('commits reset', done => { + testAction( + resetMergeRequests, + null, + mockedState, + [{ type: types.RESET_MERGE_REQUESTS }], + [], + done, + ); + }); + }); +}); diff --git a/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js b/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js new file mode 100644 index 00000000000..664d3914564 --- /dev/null +++ b/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js @@ -0,0 +1,55 @@ +import state from '~/ide/stores/modules/merge_requests/state'; +import mutations from '~/ide/stores/modules/merge_requests/mutations'; +import * as types from '~/ide/stores/modules/merge_requests/mutation_types'; +import { mergeRequests } from '../../../mock_data'; + +describe('IDE merge requests mutations', () => { + let mockedState; + + beforeEach(() => { + mockedState = state(); + }); + + describe(types.REQUEST_MERGE_REQUESTS, () => { + it('sets loading to true', () => { + mutations[types.REQUEST_MERGE_REQUESTS](mockedState); + + expect(mockedState.isLoading).toBe(true); + }); + }); + + describe(types.RECEIVE_MERGE_REQUESTS_ERROR, () => { + it('sets loading to false', () => { + mutations[types.RECEIVE_MERGE_REQUESTS_ERROR](mockedState); + + expect(mockedState.isLoading).toBe(false); + }); + }); + + describe(types.RECEIVE_MERGE_REQUESTS_SUCCESS, () => { + it('sets merge requests', () => { + gon.gitlab_url = gl.TEST_HOST; + mutations[types.RECEIVE_MERGE_REQUESTS_SUCCESS](mockedState, mergeRequests); + + expect(mockedState.mergeRequests).toEqual([ + { + id: 1, + iid: 1, + title: 'Test merge request', + projectId: 1, + projectPathWithNamespace: 'namespace/project-path', + }, + ]); + }); + }); + + describe(types.RESET_MERGE_REQUESTS, () => { + it('clears merge request array', () => { + mockedState.mergeRequests = ['test']; + + mutations[types.RESET_MERGE_REQUESTS](mockedState); + + expect(mockedState.mergeRequests).toEqual([]); + }); + }); +}); diff --git a/spec/javascripts/integrations/integration_settings_form_spec.js b/spec/javascripts/integrations/integration_settings_form_spec.js index 050b1f2074e..e07343810d2 100644 --- a/spec/javascripts/integrations/integration_settings_form_spec.js +++ b/spec/javascripts/integrations/integration_settings_form_spec.js @@ -143,6 +143,7 @@ describe('IntegrationSettingsForm', () => { error: true, message: errorMessage, service_response: 'some error', + test_failed: true, }); integrationSettingsForm.testSettings(formData) @@ -157,6 +158,27 @@ describe('IntegrationSettingsForm', () => { .catch(done.fail); }); + it('should not show error Flash with `Save anyway` action if ajax request responds with error in validation', (done) => { + const errorMessage = 'Validations failed.'; + mock.onPut(integrationSettingsForm.testEndPoint).reply(200, { + error: true, + message: errorMessage, + service_response: 'some error', + test_failed: false, + }); + + integrationSettingsForm.testSettings(formData) + .then(() => { + const $flashContainer = $('.flash-container'); + expect($flashContainer.find('.flash-text').text().trim()).toEqual('Validations failed. some error'); + expect($flashContainer.find('.flash-action')).toBeDefined(); + expect($flashContainer.find('.flash-action').text().trim()).toEqual(''); + + done(); + }) + .catch(done.fail); + }); + it('should submit form if ajax request responds without any error in test', (done) => { spyOn(integrationSettingsForm.$form, 'submit'); @@ -180,6 +202,7 @@ describe('IntegrationSettingsForm', () => { mock.onPut(integrationSettingsForm.testEndPoint).reply(200, { error: true, message: errorMessage, + test_failed: true, }); integrationSettingsForm.testSettings(formData) diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb index 023bedaaebb..f583b2021a2 100644 --- a/spec/lib/backup/repository_spec.rb +++ b/spec/lib/backup/repository_spec.rb @@ -92,7 +92,7 @@ describe Backup::Repository do end def list_repositories - Dir[SEED_STORAGE_PATH + '/*.git'] + Dir[File.join(SEED_STORAGE_PATH, '*.git')] end end diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb index 007e93c1db6..211e3aaa94b 100644 --- a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb +++ b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb @@ -299,7 +299,11 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :m let(:commits) { merge_request_diff.commits.map(&:to_hash) } let(:first_commit) { project.repository.commit(merge_request_diff.head_commit_sha) } let(:expected_commits) { commits } - let(:diffs) { first_commit.rugged_diff_from_parent.patches } + let(:diffs) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + first_commit.rugged_diff_from_parent.patches + end + end let(:expected_diffs) { [] } include_examples 'updated MR diff' @@ -309,7 +313,11 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :m let(:commits) { merge_request_diff.commits.map(&:to_hash) } let(:first_commit) { project.repository.commit(merge_request_diff.head_commit_sha) } let(:expected_commits) { commits } - let(:diffs) { first_commit.rugged_diff_from_parent.deltas } + let(:diffs) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + first_commit.rugged_diff_from_parent.deltas + end + end let(:expected_diffs) { [] } include_examples 'updated MR diff' diff --git a/spec/lib/gitlab/checks/lfs_integrity_spec.rb b/spec/lib/gitlab/checks/lfs_integrity_spec.rb index 7201e4f7bf6..ec22e3a198e 100644 --- a/spec/lib/gitlab/checks/lfs_integrity_spec.rb +++ b/spec/lib/gitlab/checks/lfs_integrity_spec.rb @@ -6,7 +6,9 @@ describe Gitlab::Checks::LfsIntegrity do let(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:newrev) do - operations = BareRepoOperations.new(repository.path) + operations = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + BareRepoOperations.new(repository.path) + end # Create a commit not pointed at by any ref to emulate being in the # pre-receive hook so that `--not --all` returns some objects diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index 4d7d6951a51..c5a4d9b4778 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -42,6 +42,10 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do it 'correctly assigns user' do expect(pipeline.builds).to all(have_attributes(user: user)) end + + it 'has pipeline iid' do + expect(pipeline.iid).to be > 0 + end end context 'when pipeline is empty' do @@ -68,6 +72,10 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do expect(pipeline.errors.to_a) .to include 'No stages / jobs for this pipeline.' end + + it 'wastes pipeline iid' do + expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0 + end end context 'when pipeline has validation errors' do @@ -87,6 +95,10 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do expect(pipeline.errors.to_a) .to include 'Failed to build the pipeline!' end + + it 'wastes pipeline iid' do + expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0 + end end context 'when there is a seed blocks present' do @@ -111,6 +123,12 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do expect(pipeline.variables.first.key).to eq 'VAR' expect(pipeline.variables.first.value).to eq '123' end + + it 'has pipeline iid' do + step.perform! + + expect(pipeline.iid).to be > 0 + end end context 'when seeds block tries to persist some resources' do @@ -121,6 +139,12 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do it 'raises exception' do expect { step.perform! }.to raise_error(ActiveRecord::RecordNotSaved) end + + it 'wastes pipeline iid' do + expect { step.perform! }.to raise_error + + expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0 + end end end @@ -132,22 +156,39 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do end end - context 'when using only/except build policies' do - let(:config) do - { rspec: { script: 'rspec', stage: 'test', only: ['master'] }, - prod: { script: 'cap prod', stage: 'deploy', only: ['tags'] } } - end + context 'when variables policy is specified' do + shared_examples_for 'a correct pipeline' do + it 'populates pipeline according to used policies' do + step.perform! - let(:pipeline) do - build(:ci_pipeline, ref: 'master', config: config) + expect(pipeline.stages.size).to eq 1 + expect(pipeline.stages.first.builds.size).to eq 1 + expect(pipeline.stages.first.builds.first.name).to eq 'rspec' + end end - it 'populates pipeline according to used policies' do - step.perform! + context 'when using only/except build policies' do + let(:config) do + { rspec: { script: 'rspec', stage: 'test', only: ['master'] }, + prod: { script: 'cap prod', stage: 'deploy', only: ['tags'] } } + end + + let(:pipeline) do + build(:ci_pipeline, ref: 'master', config: config) + end - expect(pipeline.stages.size).to eq 1 - expect(pipeline.stages.first.builds.size).to eq 1 - expect(pipeline.stages.first.builds.first.name).to eq 'rspec' + it_behaves_like 'a correct pipeline' + + context 'when variables expression is specified' do + context 'when pipeline iid is the subject' do + let(:config) do + { rspec: { script: 'rspec', only: { variables: ["$CI_PIPELINE_IID == '1'"] } }, + prod: { script: 'cap prod', only: { variables: ["$CI_PIPELINE_IID == '1000'"] } } } + end + + it_behaves_like 'a correct pipeline' + end + end end end end diff --git a/spec/lib/gitlab/conflict/file_spec.rb b/spec/lib/gitlab/conflict/file_spec.rb index 92792144429..5b343920429 100644 --- a/spec/lib/gitlab/conflict/file_spec.rb +++ b/spec/lib/gitlab/conflict/file_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::Conflict::File do let(:project) { create(:project, :repository) } let(:repository) { project.repository } - let(:rugged) { repository.rugged } + let(:rugged) { Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.rugged } } let(:their_commit) { rugged.branches['conflict-start'].target } let(:our_commit) { rugged.branches['conflict-resolvable'].target } let(:merge_request) { create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) } diff --git a/spec/lib/gitlab/cycle_analytics/usage_data_spec.rb b/spec/lib/gitlab/cycle_analytics/usage_data_spec.rb index 56a316318cb..a785b17f682 100644 --- a/spec/lib/gitlab/cycle_analytics/usage_data_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/usage_data_spec.rb @@ -3,7 +3,12 @@ require 'spec_helper' describe Gitlab::CycleAnalytics::UsageData do describe '#to_json' do before do - Timecop.freeze do + # Since git commits only have second precision, round up to the + # nearest second to ensure we have accurate median and standard + # deviation calculations. + current_time = Time.at(Time.now.to_i) + + Timecop.freeze(current_time) do user = create(:user, :admin) projects = create_list(:project, 2, :repository) @@ -37,13 +42,7 @@ describe Gitlab::CycleAnalytics::UsageData do expected_values.each_pair do |op, value| expect(stage_values).to have_key(op) - - if op == :missing - expect(stage_values[op]).to eq(value) - else - # delta is used because of git timings that Timecop does not stub - expect(stage_values[op].to_i).to be_within(5).of(value.to_i) - end + expect(stage_values[op]).to eq(value) end end end @@ -58,8 +57,8 @@ describe Gitlab::CycleAnalytics::UsageData do missing: 0 }, plan: { - average: 2, - sd: 2, + average: 1, + sd: 0, missing: 0 }, code: { diff --git a/spec/lib/gitlab/file_finder_spec.rb b/spec/lib/gitlab/file_finder_spec.rb index 07cb10e563e..d6d9e4001a3 100644 --- a/spec/lib/gitlab/file_finder_spec.rb +++ b/spec/lib/gitlab/file_finder_spec.rb @@ -3,27 +3,11 @@ require 'spec_helper' describe Gitlab::FileFinder do describe '#find' do let(:project) { create(:project, :public, :repository) } - let(:finder) { described_class.new(project, project.default_branch) } - it 'finds by name' do - results = finder.find('files') - - filename, blob = results.find { |_, blob| blob.filename == 'files/images/wm.svg' } - expect(filename).to eq('files/images/wm.svg') - expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) - expect(blob.ref).to eq(finder.ref) - expect(blob.data).not_to be_empty - end - - it 'finds by content' do - results = finder.find('files') - - filename, blob = results.find { |_, blob| blob.filename == 'CHANGELOG' } - - expect(filename).to eq('CHANGELOG') - expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) - expect(blob.ref).to eq(finder.ref) - expect(blob.data).not_to be_empty + it_behaves_like 'file finder' do + subject { described_class.new(project, project.default_branch) } + let(:expected_file_by_name) { 'files/images/wm.svg' } + let(:expected_file_by_content) { 'CHANGELOG' } end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index dd5c498706d..7a9621d9c78 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -114,7 +114,9 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'raises a no repository exception when there is no repo' do broken_repo = described_class.new('default', 'a/path.git', '') - expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Repository::NoRepository) + expect do + Gitlab::GitalyClient::StorageSettings.allow_disk_access { broken_repo.rugged } + end.to raise_error(Gitlab::Git::Repository::NoRepository) end describe 'alternates keyword argument' do @@ -124,9 +126,9 @@ describe Gitlab::Git::Repository, seed_helper: true do end it "is passed an empty array" do - expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: []) + expect(Rugged::Repository).to receive(:new).with(repository_path, alternates: []) - repository.rugged + repository_rugged end end @@ -142,10 +144,10 @@ describe Gitlab::Git::Repository, seed_helper: true do end it "is passed the relative object dir envvars after being converted to absolute ones" do - alternates = %w[foo bar baz].map { |d| File.join(repository.path, './objects', d) } - expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: alternates) + alternates = %w[foo bar baz].map { |d| File.join(repository_path, './objects', d) } + expect(Rugged::Repository).to receive(:new).with(repository_path, alternates: alternates) - repository.rugged + repository_rugged end end end @@ -156,16 +158,22 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:feature) { 'feature' } let(:feature2) { 'feature2' } + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + it "returns 'master' when master exists" do expect(repository).to receive(:branch_names).at_least(:once).and_return([feature, master]) expect(repository.discover_default_branch).to eq('master') end it "returns non-master when master exists but default branch is set to something else" do - File.write(File.join(repository.path, 'HEAD'), 'ref: refs/heads/feature') + File.write(File.join(repository_path, 'HEAD'), 'ref: refs/heads/feature') expect(repository).to receive(:branch_names).at_least(:once).and_return([feature, master]) expect(repository.discover_default_branch).to eq('feature') - File.write(File.join(repository.path, 'HEAD'), 'ref: refs/heads/master') + File.write(File.join(repository_path, 'HEAD'), 'ref: refs/heads/master') end it "returns a non-master branch when only one exists" do @@ -364,6 +372,12 @@ describe Gitlab::Git::Repository, seed_helper: true do end context '#submodules' do + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } context 'where repo has submodules' do @@ -474,8 +488,8 @@ describe Gitlab::Git::Repository, seed_helper: true do # Sanity check expect(repository.has_local_branches?).to eq(true) - FileUtils.rm_rf(File.join(repository.path, 'packed-refs')) - heads_dir = File.join(repository.path, 'refs/heads') + FileUtils.rm_rf(File.join(repository_path, 'packed-refs')) + heads_dir = File.join(repository_path, 'refs/heads') FileUtils.rm_rf(heads_dir) FileUtils.mkdir_p(heads_dir) @@ -516,10 +530,10 @@ describe Gitlab::Git::Repository, seed_helper: true do branch_name = "to-be-deleted-soon" repository.create_branch(branch_name) - expect(repository.rugged.branches[branch_name]).not_to be_nil + expect(repository_rugged.branches[branch_name]).not_to be_nil repository.delete_branch(branch_name) - expect(repository.rugged.branches[branch_name]).to be_nil + expect(repository_rugged.branches[branch_name]).to be_nil end context "when branch does not exist" do @@ -577,6 +591,12 @@ describe Gitlab::Git::Repository, seed_helper: true do shared_examples 'deleting refs' do let(:repo) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + def repo_rugged + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repo.rugged + end + end + after do ensure_seeds end @@ -584,7 +604,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'deletes the ref' do repo.delete_refs('refs/heads/feature') - expect(repo.rugged.references['refs/heads/feature']).to be_nil + expect(repo_rugged.references['refs/heads/feature']).to be_nil end it 'deletes all refs' do @@ -592,7 +612,7 @@ describe Gitlab::Git::Repository, seed_helper: true do repo.delete_refs(*refs) refs.each do |ref| - expect(repo.rugged.references[ref]).to be_nil + expect(repo_rugged.references[ref]).to be_nil end end @@ -615,7 +635,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#branch_names_contains_sha' do - let(:head_id) { repository.rugged.head.target.oid } + let(:head_id) { repository_rugged.head.target.oid } let(:new_branch) { head_id } let(:utf8_branch) { 'branch-é' } @@ -699,7 +719,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'fetches a repository as a mirror remote' do subject - expect(refs(new_repository.path)).to eq(refs(repository.path)) + expect(refs(new_repository_path)).to eq(refs(repository_path)) end context 'with keep-around refs' do @@ -708,15 +728,15 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } before do - repository.rugged.references.create(keep_around_ref, sha, force: true) - repository.rugged.references.create(tmp_ref, sha, force: true) + repository_rugged.references.create(keep_around_ref, sha, force: true) + repository_rugged.references.create(tmp_ref, sha, force: true) end it 'includes the temporary and keep-around refs' do subject - expect(refs(new_repository.path)).to include(keep_around_ref) - expect(refs(new_repository.path)).to include(tmp_ref) + expect(refs(new_repository_path)).to include(keep_around_ref) + expect(refs(new_repository_path)).to include(tmp_ref) end end end @@ -728,6 +748,12 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with gitaly enabled', :skip_gitaly_mock do it_behaves_like 'repository mirror fecthing' end + + def new_repository_path + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + new_repository.path + end + end end describe '#remote_tags' do @@ -739,10 +765,17 @@ describe Gitlab::Git::Repository, seed_helper: true do Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') end + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + subject { repository.remote_tags(remote_name) } before do - repository.add_remote(remote_name, remote_repository.path) + remote_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { remote_repository.path } + repository.add_remote(remote_name, remote_repository_path) remote_repository.add_tag(tag_name, user: user, target: target_commit_id) end @@ -975,8 +1008,10 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:options) { { ref: 'master', path: ['PROCESS.md', 'README.md'] } } def commit_files(commit) - commit.rugged_diff_from_parent.deltas.flat_map do |delta| - [delta.old_file[:path], delta.new_file[:path]].uniq.compact + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + commit.rugged_diff_from_parent.deltas.flat_map do |delta| + [delta.old_file[:path], delta.new_file[:path]].uniq.compact + end end end @@ -1019,6 +1054,12 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe "#rugged_commits_between" do + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + context 'two SHAs' do let(:first_sha) { 'b0e52af38d7ea43cf41d8a6f2471351ac036d6c9' } let(:second_sha) { '0e50ec4d3c7ce42ab74dda1d422cb2cbffe1e326' } @@ -1363,7 +1404,7 @@ describe Gitlab::Git::Repository, seed_helper: true do allow(ref).to receive(:target) { raise Rugged::ReferenceError } branches = double() allow(branches).to receive(:each) { [ref].each } - allow(repository.rugged).to receive(:branches) { branches } + allow(repository_rugged).to receive(:branches) { branches } expect(subject).to be_empty end @@ -1661,6 +1702,12 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#batch_existence' do let(:refs) { ['deadbeef', SeedRepo::RubyBlob::ID, '909e6157199'] } + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + it 'returns existing refs back' do result = repository.batch_existence(refs) @@ -1840,7 +1887,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'when the branch exists' do context 'when the commit does not exist locally' do let(:source_branch) { 'new-branch-for-fetch-source-branch' } - let(:source_rugged) { source_repository.rugged } + let(:source_rugged) { Gitlab::GitalyClient::StorageSettings.allow_disk_access { source_repository.rugged } } let(:new_oid) { new_commit_edit_old_file(source_rugged).oid } before do @@ -1898,7 +1945,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it "removes the branch from the repo" do repository.rm_branch(branch_name, user: user) - expect(repository.rugged.branches[branch_name]).to be_nil + expect(repository_rugged.branches[branch_name]).to be_nil end end @@ -1930,7 +1977,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#write_config' do before do - repository.rugged.config["gitlab.fullpath"] = repository.path + repository_rugged.config["gitlab.fullpath"] = repository_path end shared_examples 'writing repo config' do @@ -1938,7 +1985,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'writes it to disk' do repository.write_config(full_path: "not-the/real-path.git") - config = File.read(File.join(repository.path, "config")) + config = File.read(File.join(repository_path, "config")) expect(config).to include("[gitlab]") expect(config).to include("fullpath = not-the/real-path.git") @@ -1949,10 +1996,10 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'does not write it to disk' do repository.write_config(full_path: "") - config = File.read(File.join(repository.path, "config")) + config = File.read(File.join(repository_path, "config")) expect(config).to include("[gitlab]") - expect(config).to include("fullpath = #{repository.path}") + expect(config).to include("fullpath = #{repository_path}") end end end @@ -2173,7 +2220,11 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#gitlab_projects' do subject { repository.gitlab_projects } - it { expect(subject.shard_path).to eq(storage_path) } + it do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + expect(subject.shard_path).to eq(storage_path) + end + end it { expect(subject.repository_relative_path).to eq(repository.relative_path) } end @@ -2189,7 +2240,7 @@ describe Gitlab::Git::Repository, seed_helper: true do repository.bundle_to_disk(save_path) success = system( - *%W(#{Gitlab.config.git.bin_path} -C #{repository.path} bundle verify #{save_path}), + *%W(#{Gitlab.config.git.bin_path} -C #{repository_path} bundle verify #{save_path}), [:out, :err] => '/dev/null' ) expect(success).to be true @@ -2231,7 +2282,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'creates a symlink to the global hooks dir' do imported_repo.create_from_bundle(bundle_path) - hooks_path = File.join(imported_repo.path, 'hooks') + hooks_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { File.join(imported_repo.path, 'hooks') } expect(File.readlink(hooks_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) end @@ -2360,7 +2411,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#clean_stale_repository_files' do - let(:worktree_path) { File.join(repository.path, 'worktrees', 'delete-me') } + let(:worktree_path) { File.join(repository_path, 'worktrees', 'delete-me') } it 'cleans up the files' do repository.with_worktree(worktree_path, 'master', env: ENV) do @@ -2507,7 +2558,7 @@ describe Gitlab::Git::Repository, seed_helper: true do def create_remote_branch(repository, remote_name, branch_name, source_branch_name) source_branch = repository.branches.find { |branch| branch.name == source_branch_name } - rugged = repository.rugged + rugged = repository_rugged rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha) end @@ -2586,4 +2637,16 @@ describe Gitlab::Git::Repository, seed_helper: true do line.split("\t").last end end + + def repository_rugged + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged + end + end + + def repository_path + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.path + end + end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 74e7a45fd6c..2aebdc57f7c 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -242,6 +242,7 @@ Ci::Pipeline: - config_source - failure_reason - protected +- iid Ci::Stage: - id - name diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index e3f705d2299..50224bde722 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -22,47 +22,57 @@ describe Gitlab::ProjectSearchResults do it { expect(results.query).to eq('hello world') } end - describe 'blob search' do - let(:project) { create(:project, :public, :repository) } - - subject(:results) { described_class.new(user, project, 'files').objects('blobs') } - - context 'when repository is disabled' do - let(:project) { create(:project, :public, :repository, :repository_disabled) } + shared_examples 'general blob search' do |entity_type, blob_kind| + let(:query) { 'files' } + subject(:results) { described_class.new(user, project, query).objects(blob_type) } - it 'hides blobs from members' do + context "when #{entity_type} is disabled" do + let(:project) { disabled_project } + it "hides #{blob_kind} from members" do project.add_reporter(user) is_expected.to be_empty end - it 'hides blobs from non-members' do + it "hides #{blob_kind} from non-members" do is_expected.to be_empty end end - context 'when repository is internal' do - let(:project) { create(:project, :public, :repository, :repository_private) } + context "when #{entity_type} is internal" do + let(:project) { private_project } - it 'finds blobs for members' do + it "finds #{blob_kind} for members" do project.add_reporter(user) is_expected.not_to be_empty end - it 'hides blobs from non-members' do + it "hides #{blob_kind} from non-members" do is_expected.to be_empty end end it 'finds by name' do - expect(results.map(&:first)).to include('files/images/wm.svg') + expect(results.map(&:first)).to include(expected_file_by_name) end it 'finds by content' do - blob = results.select { |result| result.first == "CHANGELOG" }.flatten.last + blob = results.select { |result| result.first == expected_file_by_content }.flatten.last - expect(blob.filename).to eq("CHANGELOG") + expect(blob.filename).to eq(expected_file_by_content) + end + end + + describe 'blob search' do + let(:project) { create(:project, :public, :repository) } + + it_behaves_like 'general blob search', 'repository', 'blobs' do + let(:blob_type) { 'blobs' } + let(:disabled_project) { create(:project, :public, :repository, :repository_disabled) } + let(:private_project) { create(:project, :public, :repository, :repository_private) } + let(:expected_file_by_name) { 'files/images/wm.svg' } + let(:expected_file_by_content) { 'CHANGELOG' } end describe 'parsing results' do @@ -189,40 +199,18 @@ describe Gitlab::ProjectSearchResults do describe 'wiki search' do let(:project) { create(:project, :public, :wiki_repo) } let(:wiki) { build(:project_wiki, project: project) } - let!(:wiki_page) { wiki.create_page('Title', 'Content') } - - subject(:results) { described_class.new(user, project, 'Content').objects('wiki_blobs') } - - context 'when wiki is disabled' do - let(:project) { create(:project, :public, :wiki_repo, :wiki_disabled) } - it 'hides wiki blobs from members' do - project.add_reporter(user) - - is_expected.to be_empty - end - - it 'hides wiki blobs from non-members' do - is_expected.to be_empty - end - end - - context 'when wiki is internal' do - let(:project) { create(:project, :public, :wiki_repo, :wiki_private) } - - it 'finds wiki blobs for guest' do - project.add_guest(user) - - is_expected.not_to be_empty - end - - it 'hides wiki blobs from non-members' do - is_expected.to be_empty - end + before do + wiki.create_page('Files/Title', 'Content') + wiki.create_page('CHANGELOG', 'Files example') end - it 'finds by content' do - expect(results).to include("master:Title.md\x001\x00Content\n") + it_behaves_like 'general blob search', 'wiki', 'wiki blobs' do + let(:blob_type) { 'wiki_blobs' } + let(:disabled_project) { create(:project, :public, :wiki_repo, :wiki_disabled) } + let(:private_project) { create(:project, :public, :wiki_repo, :wiki_private) } + let(:expected_file_by_name) { 'Files/Title.md' } + let(:expected_file_by_content) { 'CHANGELOG.md' } end end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index a3b3dc3be6d..81dbbb962dd 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::UrlBlocker do describe '#blocked_url?' do - let(:valid_ports) { Project::VALID_IMPORT_PORTS } + let(:ports) { Project::VALID_IMPORT_PORTS } it 'allows imports from configured web host and port' do import_url = "http://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/t.git" @@ -19,7 +19,13 @@ describe Gitlab::UrlBlocker do end it 'returns true for bad port' do - expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git', valid_ports: valid_ports)).to be true + expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git', ports: ports)).to be true + end + + it 'returns true for bad protocol' do + expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', protocols: ['https'])).to be false + expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false + expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', protocols: ['http'])).to be true end it 'returns true for alternative version of 127.0.0.1 (0177.1)' do diff --git a/spec/lib/gitlab/wiki_file_finder_spec.rb b/spec/lib/gitlab/wiki_file_finder_spec.rb new file mode 100644 index 00000000000..025d1203dc5 --- /dev/null +++ b/spec/lib/gitlab/wiki_file_finder_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Gitlab::WikiFileFinder do + describe '#find' do + let(:project) { create(:project, :public, :wiki_repo) } + let(:wiki) { build(:project_wiki, project: project) } + + before do + wiki.create_page('Files/Title', 'Content') + wiki.create_page('CHANGELOG', 'Files example') + end + + it_behaves_like 'file finder' do + subject { described_class.new(project, project.wiki.default_branch) } + + let(:expected_file_by_name) { 'Files/Title.md' } + let(:expected_file_by_content) { 'CHANGELOG.md' } + end + end +end diff --git a/spec/lib/omni_auth/strategies/jwt_spec.rb b/spec/lib/omni_auth/strategies/jwt_spec.rb index 23485fbcb18..88d6d0b559a 100644 --- a/spec/lib/omni_auth/strategies/jwt_spec.rb +++ b/spec/lib/omni_auth/strategies/jwt_spec.rb @@ -43,7 +43,7 @@ describe OmniAuth::Strategies::Jwt do end it 'raises error' do - expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::JWT::ClaimInvalid) + expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid) end end @@ -61,7 +61,7 @@ describe OmniAuth::Strategies::Jwt do end it 'raises error' do - expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::JWT::ClaimInvalid) + expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid) end end @@ -80,7 +80,7 @@ describe OmniAuth::Strategies::Jwt do end it 'raises error' do - expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::JWT::ClaimInvalid) + expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid) end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 77179028ede..66c9708b4cf 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -148,10 +148,9 @@ describe Ci::Build do end context 'when there are runners' do - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, :project, projects: [build.project]) } before do - build.project.runners << runner runner.update_attributes(contacted_at: 1.second.ago) end @@ -1388,12 +1387,7 @@ describe Ci::Build do it { is_expected.to be_truthy } context "and there are specific runner" do - let(:runner) { create(:ci_runner, contacted_at: 1.second.ago) } - - before do - build.project.runners << runner - runner.save - end + let!(:runner) { create(:ci_runner, :project, projects: [build.project], contacted_at: 1.second.ago) } it { is_expected.to be_falsey } end @@ -1565,6 +1559,7 @@ describe Ci::Build do { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true }, { key: 'CI_PROJECT_URL', value: project.web_url, public: true }, { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true }, + { key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true }, { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true }, { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true }, { key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true }, diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index e4f4c62bd22..24692ebb9a3 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -35,6 +35,16 @@ describe Ci::Pipeline, :mailer do end end + describe 'modules' do + it_behaves_like 'AtomicInternalId', validate_presence: false do + let(:internal_id_attribute) { :iid } + let(:instance) { build(:ci_pipeline) } + let(:scope) { :project } + let(:scope_attrs) { { project: instance.project } } + let(:usage) { :ci_pipelines } + end + end + describe '#source' do context 'when creating new pipeline' do let(:pipeline) do @@ -195,7 +205,8 @@ describe Ci::Pipeline, :mailer do it 'includes all predefined variables in a valid order' do keys = subject.map { |variable| variable[:key] } - expect(keys).to eq %w[CI_CONFIG_PATH + expect(keys).to eq %w[CI_PIPELINE_IID + CI_CONFIG_PATH CI_PIPELINE_SOURCE CI_COMMIT_MESSAGE CI_COMMIT_TITLE @@ -1589,7 +1600,7 @@ describe Ci::Pipeline, :mailer do context 'when pipeline is not stuck' do before do - create(:ci_runner, :shared, :online) + create(:ci_runner, :instance, :online) end it 'is not stuck' do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 0fbc934f669..0f072aa1719 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -21,60 +21,58 @@ describe Ci::Runner do end end - context 'either_projects_or_group' do + context '#exactly_one_group' do let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } - it 'disallows assigning to a group if already assigned to a group' do - runner = create(:ci_runner, groups: [group]) - + it 'disallows assigning group if already assigned to a group' do runner.groups << build(:group) expect(runner).not_to be_valid - expect(runner.errors.full_messages).to eq ['Runner can only be assigned to one group'] + expect(runner.errors.full_messages).to include('Runner needs to be assigned to exactly one group') end + end - it 'disallows assigning to a group if already assigned to a project' do - project = create(:project) - runner = create(:ci_runner, projects: [project]) + context 'runner_type validations' do + set(:group) { create(:group) } + set(:project) { create(:project) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let(:project_runner) { create(:ci_runner, :project, projects: [project]) } + let(:instance_runner) { create(:ci_runner, :instance) } - runner.groups << build(:group) + it 'disallows assigning group to project_type runner' do + project_runner.groups << build(:group) - expect(runner).not_to be_valid - expect(runner.errors.full_messages).to eq ['Runner can only be assigned either to projects or to a group'] + expect(project_runner).not_to be_valid + expect(project_runner.errors.full_messages).to include('Runner cannot have groups assigned') end - it 'disallows assigning to a project if already assigned to a group' do - runner = create(:ci_runner, groups: [group]) - - runner.projects << build(:project) + it 'disallows assigning group to instance_type runner' do + instance_runner.groups << build(:group) - expect(runner).not_to be_valid - expect(runner.errors.full_messages).to eq ['Runner can only be assigned either to projects or to a group'] + expect(instance_runner).not_to be_valid + expect(instance_runner.errors.full_messages).to include('Runner cannot have groups assigned') end - it 'allows assigning to a group if not assigned to a group nor a project' do - runner = create(:ci_runner) - - runner.groups << build(:group) + it 'disallows assigning project to group_type runner' do + group_runner.projects << build(:project) - expect(runner).to be_valid + expect(group_runner).not_to be_valid + expect(group_runner.errors.full_messages).to include('Runner cannot have projects assigned') end - it 'allows assigning to a project if not assigned to a group nor a project' do - runner = create(:ci_runner) - - runner.projects << build(:project) + it 'disallows assigning project to instance_type runner' do + instance_runner.projects << build(:project) - expect(runner).to be_valid + expect(instance_runner).not_to be_valid + expect(instance_runner.errors.full_messages).to include('Runner cannot have projects assigned') end - it 'allows assigning to a project if already assigned to a project' do - project = create(:project) - runner = create(:ci_runner, projects: [project]) - - runner.projects << build(:project) + it 'should fail to save a group assigned to a project runner even if the runner is already saved' do + group_runner - expect(runner).to be_valid + expect { create(:group, runners: [project_runner]) } + .to raise_error(ActiveRecord::RecordInvalid) end end end @@ -110,17 +108,12 @@ describe Ci::Runner do describe '.shared' do let(:group) { create(:group) } let(:project) { create(:project) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let!(:project_runner) { create(:ci_runner, :project, projects: [project]) } + let!(:shared_runner) { create(:ci_runner, :instance) } - it 'returns the shared group runner' do - runner = create(:ci_runner, :shared, groups: [group]) - - expect(described_class.shared).to eq [runner] - end - - it 'returns the shared project runner' do - runner = create(:ci_runner, :shared, projects: [project]) - - expect(described_class.shared).to eq [runner] + it 'returns only shared runners' do + expect(described_class.shared).to contain_exactly(shared_runner) end end @@ -128,11 +121,11 @@ describe Ci::Runner do it 'returns the specific project runner' do # own specific_project = create(:project) - specific_runner = create(:ci_runner, :specific, projects: [specific_project]) + specific_runner = create(:ci_runner, :project, projects: [specific_project]) # other other_project = create(:project) - create(:ci_runner, :specific, projects: [other_project]) + create(:ci_runner, :project, projects: [other_project]) expect(described_class.belonging_to_project(specific_project.id)).to eq [specific_runner] end @@ -141,17 +134,17 @@ describe Ci::Runner do describe '.belonging_to_parent_group_of_project' do let(:project) { create(:project, group: group) } let(:group) { create(:group) } - let(:runner) { create(:ci_runner, :specific, groups: [group]) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } let!(:unrelated_group) { create(:group) } let!(:unrelated_project) { create(:project, group: unrelated_group) } - let!(:unrelated_runner) { create(:ci_runner, :specific, groups: [unrelated_group]) } + let!(:unrelated_runner) { create(:ci_runner, :group, groups: [unrelated_group]) } it 'returns the specific group runner' do expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner) end context 'with a parent group with a runner', :nested_groups do - let(:runner) { create(:ci_runner, :specific, groups: [parent_group]) } + let(:runner) { create(:ci_runner, :group, groups: [parent_group]) } let(:project) { create(:project, group: group) } let(:group) { create(:group, parent: parent_group) } let(:parent_group) { create(:group) } @@ -167,13 +160,13 @@ describe Ci::Runner do # group specific group = create(:group) project = create(:project, group: group) - group_runner = create(:ci_runner, :specific, groups: [group]) + group_runner = create(:ci_runner, :group, groups: [group]) # project specific - project_runner = create(:ci_runner, :specific, projects: [project]) + project_runner = create(:ci_runner, :project, projects: [project]) # globally shared - shared_runner = create(:ci_runner, :shared) + shared_runner = create(:ci_runner, :instance) expect(described_class.owned_or_shared(project.id)).to contain_exactly( group_runner, project_runner, shared_runner @@ -183,31 +176,32 @@ describe Ci::Runner do describe '#display_name' do it 'returns the description if it has a value' do - runner = FactoryBot.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') + runner = build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') expect(runner.display_name).to eq 'Linux/Ruby-1.9.3-p448' end it 'returns the token if it does not have a description' do - runner = FactoryBot.create(:ci_runner) + runner = create(:ci_runner) expect(runner.display_name).to eq runner.description end it 'returns the token if the description is an empty string' do - runner = FactoryBot.build(:ci_runner, description: '', token: 'token') + runner = build(:ci_runner, description: '', token: 'token') expect(runner.display_name).to eq runner.token end end describe '#assign_to' do - let!(:project) { FactoryBot.create(:project) } + let(:project) { create(:project) } subject { runner.assign_to(project) } context 'with shared_runner' do - let!(:runner) { FactoryBot.create(:ci_runner, :shared) } + let(:runner) { create(:ci_runner, :instance) } it 'transitions shared runner to project runner and assigns project' do - subject + expect(subject).to be_truthy + expect(runner).to be_specific expect(runner).to be_project_type expect(runner.projects).to eq([project]) @@ -216,7 +210,8 @@ describe Ci::Runner do end context 'with group runner' do - let!(:runner) { FactoryBot.create(:ci_runner, runner_type: :group_type) } + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } it 'raises an error' do expect { subject } @@ -229,15 +224,15 @@ describe Ci::Runner do subject { described_class.online } before do - @runner1 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.year.ago) - @runner2 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago) + @runner1 = create(:ci_runner, :instance, contacted_at: 1.year.ago) + @runner2 = create(:ci_runner, :instance, contacted_at: 1.second.ago) end it { is_expected.to eq([@runner2])} end describe '#online?' do - let(:runner) { FactoryBot.create(:ci_runner, :shared) } + let(:runner) { create(:ci_runner, :instance) } subject { runner.online? } @@ -307,21 +302,20 @@ describe Ci::Runner do end describe '#can_pick?' do - let(:pipeline) { create(:ci_pipeline) } + set(:pipeline) { create(:ci_pipeline) } let(:build) { create(:ci_build, pipeline: pipeline) } - let(:runner) { create(:ci_runner, tag_list: tag_list, run_untagged: run_untagged) } + let(:runner_project) { build.project } + let(:runner) { create(:ci_runner, :project, projects: [runner_project], tag_list: tag_list, run_untagged: run_untagged) } let(:tag_list) { [] } let(:run_untagged) { true } subject { runner.can_pick?(build) } - before do - build.project.runners << runner - end - context 'a different runner' do + let(:other_project) { create(:project) } + let(:other_runner) { create(:ci_runner, :project, projects: [other_project], tag_list: tag_list, run_untagged: run_untagged) } + it 'cannot handle builds' do - other_runner = create(:ci_runner) expect(other_runner.can_pick?(build)).to be_falsey end end @@ -375,18 +369,14 @@ describe Ci::Runner do end context 'when runner is shared' do - let(:runner) { create(:ci_runner, :shared) } - - before do - build.project.runners = [] - end + let(:runner) { create(:ci_runner, :instance) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy end context 'when runner is locked' do - let(:runner) { create(:ci_runner, :shared, locked: true) } + let(:runner) { create(:ci_runner, :instance, locked: true) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy @@ -401,10 +391,8 @@ describe Ci::Runner do end end - context 'when runner is not assigned to a project' do - before do - build.project.runners = [] - end + context 'when runner is assigned to another project' do + let(:runner_project) { create(:project) } it 'cannot handle builds' do expect(runner.can_pick?(build)).to be_falsey @@ -412,10 +400,8 @@ describe Ci::Runner do end context 'when runner is assigned to a group' do - before do - build.project.runners = [] - runner.groups << create(:group, projects: [build.project]) - end + let(:group) { create(:group, projects: [build.project]) } + let(:runner) { create(:ci_runner, :group, tag_list: tag_list, run_untagged: run_untagged, groups: [group]) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy @@ -469,7 +455,7 @@ describe Ci::Runner do end describe '#status' do - let(:runner) { FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago) } + let(:runner) { create(:ci_runner, :instance, contacted_at: 1.second.ago) } subject { runner.status } @@ -626,12 +612,13 @@ describe Ci::Runner do end describe '.assignable_for' do - let!(:unlocked_project_runner) { create(:ci_runner, runner_type: :project_type, projects: [project]) } - let!(:locked_project_runner) { create(:ci_runner, runner_type: :project_type, locked: true, projects: [project]) } - let!(:group_runner) { create(:ci_runner, runner_type: :group_type) } - let!(:instance_runner) { create(:ci_runner, :shared) } let(:project) { create(:project) } + let(:group) { create(:group) } let(:another_project) { create(:project) } + let!(:unlocked_project_runner) { create(:ci_runner, :project, projects: [project]) } + let!(:locked_project_runner) { create(:ci_runner, :project, locked: true, projects: [project]) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let!(:instance_runner) { create(:ci_runner, :instance) } context 'with already assigned project' do subject { described_class.assignable_for(project) } @@ -651,19 +638,16 @@ describe Ci::Runner do describe "belongs_to_one_project?" do it "returns false if there are two projects runner assigned to" do - runner = FactoryBot.create(:ci_runner) - project = FactoryBot.create(:project) - project1 = FactoryBot.create(:project) - project.runners << runner - project1.runners << runner + project1 = create(:project) + project2 = create(:project) + runner = create(:ci_runner, :project, projects: [project1, project2]) expect(runner.belongs_to_one_project?).to be_falsey end it "returns true" do - runner = FactoryBot.create(:ci_runner) - project = FactoryBot.create(:project) - project.runners << runner + project = create(:project) + runner = create(:ci_runner, :project, projects: [project]) expect(runner.belongs_to_one_project?).to be_truthy end @@ -713,21 +697,21 @@ describe Ci::Runner do subject { runner.assigned_to_group? } context 'when project runner' do - let(:runner) { create(:ci_runner, description: 'Project runner', projects: [project]) } + let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } let(:project) { create(:project) } it { is_expected.to be_falsey } end context 'when shared runner' do - let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') } + let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') } it { is_expected.to be_falsey } end context 'when group runner' do let(:group) { create(:group) } - let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) } + let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } it { is_expected.to be_truthy } end @@ -737,18 +721,18 @@ describe Ci::Runner do subject { runner.assigned_to_project? } context 'when group runner' do - let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) } + let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } let(:group) { create(:group) } it { is_expected.to be_falsey } end context 'when shared runner' do - let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') } + let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') } it { is_expected.to be_falsey } end context 'when project runner' do - let(:runner) { create(:ci_runner, description: 'Group runner', projects: [project]) } + let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } let(:project) { create(:project) } it { is_expected.to be_truthy } @@ -780,4 +764,17 @@ describe Ci::Runner do end end end + + describe 'project runner without projects is destroyable' do + subject { create(:ci_runner, :project, :without_projects) } + + it 'does not have projects' do + expect(subject.runner_projects).to be_empty + end + + it 'can be destroyed' do + subject + expect { subject.destroy }.to change { described_class.count }.by(-1) + end + end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index bd6bf5b0712..1cfd526834c 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -12,6 +12,7 @@ describe Issuable do it { is_expected.to belong_to(:author) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } + it { is_expected.to have_many(:labels) } context 'Notes' do let!(:note) { create(:note, noteable: issue, project: issue.project) } @@ -274,8 +275,8 @@ describe Issuable do it 'skips coercion for not Integer values' do expect { issue.time_estimate = nil }.to change { issue.time_estimate }.to(nil) - expect { issue.time_estimate = 'invalid time' }.not_to raise_error(StandardError) - expect { issue.time_estimate = 22.33 }.not_to raise_error(StandardError) + expect { issue.time_estimate = 'invalid time' }.not_to raise_error + expect { issue.time_estimate = 22.33 }.not_to raise_error end end diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 4570dbb1d8e..f2a3df50c1a 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -94,6 +94,7 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do end it { expect(instance.result).to be_nil } + it { expect(reactive_cache_alive?(instance)).to be_falsy } end describe '#exclusively_update_reactive_cache!' do diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index aee70bcfb29..e01906f4b6c 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -20,6 +20,7 @@ describe Deployment do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:deployment) } + let(:scope) { :project } let(:scope_attrs) { { project: instance.project } } let(:usage) { :deployments } end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 128acf83686..e818fbeb9cf 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -17,6 +17,7 @@ describe Issue do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:issue) } + let(:scope) { :project } let(:scope_attrs) { { project: instance.project } } let(:usage) { :issues } end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 9ffa91fc265..65cc9372cbe 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -84,6 +84,7 @@ describe MergeRequest do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:merge_request) } + let(:scope) { :target_project } let(:scope_attrs) { { project: instance.target_project } } let(:usage) { :merge_requests } end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 4bb9717d33e..204d6b47832 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -6,6 +6,7 @@ describe Milestone do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:milestone, project: build(:project), group: nil) } + let(:scope) { :project } let(:scope_attrs) { { project: instance.project } } let(:usage) { :milestones } end @@ -15,6 +16,7 @@ describe Milestone do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:milestone, project: nil, group: build(:group)) } + let(:scope) { :group } let(:scope_attrs) { { namespace: instance.group } } let(:usage) { :milestones } end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index af2240f4f89..9a76452a808 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1177,8 +1177,8 @@ describe Project do describe '#any_runners?' do context 'shared runners' do let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) } - let(:specific_runner) { create(:ci_runner) } - let(:shared_runner) { create(:ci_runner, :shared) } + let(:specific_runner) { create(:ci_runner, :project, projects: [project]) } + let(:shared_runner) { create(:ci_runner, :instance) } context 'for shared runners disabled' do let(:shared_runners_enabled) { false } @@ -1188,7 +1188,7 @@ describe Project do end it 'has a specific runner' do - project.runners << specific_runner + specific_runner expect(project.any_runners?).to be_truthy end @@ -1200,13 +1200,13 @@ describe Project do end it 'checks the presence of specific runner' do - project.runners << specific_runner + specific_runner expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy end it 'returns false if match cannot be found' do - project.runners << specific_runner + specific_runner expect(project.any_runners? { false }).to be_falsey end @@ -1238,7 +1238,7 @@ describe Project do context 'group runners' do let(:project) { create(:project, group_runners_enabled: group_runners_enabled) } let(:group) { create(:group, projects: [project]) } - let(:group_runner) { create(:ci_runner, groups: [group]) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } context 'for group runners disabled' do let(:group_runners_enabled) { false } @@ -1279,7 +1279,7 @@ describe Project do end describe '#shared_runners' do - let!(:runner) { create(:ci_runner, :shared) } + let!(:runner) { create(:ci_runner, :instance) } subject { project.shared_runners } diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index a80800c6c92..1d94abe4195 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -12,8 +12,8 @@ describe RemoteMirror do context 'with an invalid URL' do it 'should not be valid' do remote_mirror = build(:remote_mirror, url: 'ftp://invalid.invalid') + expect(remote_mirror).not_to be_valid - expect(remote_mirror.errors[:url].size).to eq(2) end end end diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index 6e30798356c..a0c93c531ea 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -5,6 +5,9 @@ RSpec.describe Timelog do let(:issue) { create(:issue) } let(:merge_request) { create(:merge_request) } + it { is_expected.to belong_to(:issue).touch(true) } + it { is_expected.to belong_to(:merge_request).touch(true) } + it { is_expected.to be_valid } it { is_expected.to validate_presence_of(:time_spent) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 16b409844fa..09dfeae6377 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1858,13 +1858,10 @@ describe User do describe '#ci_owned_runners' do let(:user) { create(:user) } - let(:runner_1) { create(:ci_runner) } - let(:runner_2) { create(:ci_runner) } + let!(:project) { create(:project) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } context 'without any projects nor groups' do - let!(:project) { create(:project, runners: [runner_1]) } - let!(:group) { create(:group) } - it 'does not load' do expect(user.ci_owned_runners).to be_empty end @@ -1872,38 +1869,40 @@ describe User do context 'with personal projects runners' do let(:namespace) { create(:namespace, owner: user) } - let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } + let!(:project) { create(:project, namespace: namespace) } it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_1) + expect(user.ci_owned_runners).to contain_exactly(runner) end end context 'with personal group runner' do - let!(:project) { create(:project, runners: [runner_1]) } + let!(:project) { create(:project) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:group) do - create(:group, runners: [runner_2]).tap do |group| + create(:group).tap do |group| group.add_owner(user) end end it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_2) + expect(user.ci_owned_runners).to contain_exactly(group_runner) end end context 'with personal project and group runner' do let(:namespace) { create(:namespace, owner: user) } - let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } + let!(:project) { create(:project, namespace: namespace) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:group) do - create(:group, runners: [runner_2]).tap do |group| + create(:group).tap do |group| group.add_owner(user) end end it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_1, runner_2) + expect(user.ci_owned_runners).to contain_exactly(runner, group_runner) end end @@ -1914,7 +1913,7 @@ describe User do end it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_1) + expect(user.ci_owned_runners).to contain_exactly(runner) end end @@ -1931,7 +1930,7 @@ describe User do context 'with groups projects runners' do let(:group) { create(:group) } - let!(:project) { create(:project, group: group, runners: [runner_1]) } + let!(:project) { create(:project, group: group) } def add_user(access) group.add_user(user, access) @@ -1941,11 +1940,8 @@ describe User do end context 'with groups runners' do - let!(:group) do - create(:group, runners: [runner_1]).tap do |group| - group.add_owner(user) - end - end + let!(:runner) { create(:ci_runner, :group, groups: [group]) } + let!(:group) { create(:group) } def add_user(access) group.add_user(user, access) @@ -1955,7 +1951,7 @@ describe User do end context 'with other projects runners' do - let!(:project) { create(:project, runners: [runner_1]) } + let!(:project) { create(:project) } def add_user(access) project.add_role(user, access) @@ -1968,7 +1964,7 @@ describe User do let(:group) { create(:group) } let(:another_user) { create(:user) } let(:subgroup) { create(:group, parent: group) } - let!(:project) { create(:project, group: subgroup, runners: [runner_1]) } + let!(:project) { create(:project, group: subgroup) } def add_user(access) group.add_user(user, access) diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index f246bb79ab7..cd43bec35df 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -304,7 +304,7 @@ describe API::CommitStatuses do it 'responds with bad request status and validation errors' do expect(response).to have_gitlab_http_status(400) expect(json_response['message']['target_url']) - .to include 'must be a valid URL' + .to include 'is blocked: Only allowed protocols are http, https' end end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 3106083293f..4181f4ebbbe 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1351,19 +1351,25 @@ describe API::Issues do expect(json_response['labels']).to eq([label.title]) end - it 'removes all labels' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: '' + it 'removes all labels and touches the record' do + Timecop.travel(1.minute.from_now) do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: '' + end expect(response).to have_gitlab_http_status(200) expect(json_response['labels']).to eq([]) + expect(json_response['updated_at']).to be > Time.now end - it 'updates labels' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), + it 'updates labels and touches the record' do + Timecop.travel(1.minute.from_now) do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: 'foo,bar' + end expect(response).to have_gitlab_http_status(200) expect(json_response['labels']).to include 'foo' expect(json_response['labels']).to include 'bar' + expect(json_response['updated_at']).to be > Time.now end it 'allows special label names' do diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 6aadf839dbd..319ac389083 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -111,11 +111,13 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end context 'when tags are not provided' do - it 'returns 404 error' do + it 'returns 400 error' do post api('/runners'), token: registration_token, run_untagged: false - expect(response).to have_gitlab_http_status 404 + expect(response).to have_gitlab_http_status 400 + expect(json_response['message']).to include( + 'tags_list' => ['can not be empty when runner is not allowed to pick untagged jobs']) end end end @@ -262,16 +264,12 @@ describe API::Runner, :clean_gitlab_redis_shared_state do describe '/api/v4/jobs' do let(:project) { create(:project, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') } - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:job) do create(:ci_build, :artifacts, :extended_options, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, commands: "ls\ndate") end - before do - project.runners << runner - end - describe 'POST /api/v4/jobs/request' do let!(:last_update) {} let!(:new_update) { } @@ -379,7 +377,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end context 'when shared runner requests job for project without shared_runners_enabled' do - let(:runner) { create(:ci_runner, :shared) } + let(:runner) { create(:ci_runner, :instance) } it_behaves_like 'no jobs available' end @@ -724,7 +722,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end context 'when runner specifies lower timeout' do - let(:runner) { create(:ci_runner, maximum_timeout: 1000) } + let(:runner) { create(:ci_runner, :project, maximum_timeout: 1000, projects: [project]) } it 'contains info about timeout overridden by runner' do request_job @@ -735,7 +733,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end context 'when runner specifies bigger timeout' do - let(:runner) { create(:ci_runner, maximum_timeout: 2000) } + let(:runner) { create(:ci_runner, :project, maximum_timeout: 2000, projects: [project]) } it 'contains info about timeout not overridden by runner' do request_job diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index c7587c877fc..0c7937feed6 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -11,23 +11,10 @@ describe API::Runners do let(:group) { create(:group).tap { |group| group.add_owner(user) } } let(:group2) { create(:group).tap { |group| group.add_owner(user) } } - let!(:shared_runner) { create(:ci_runner, :shared, description: 'Shared runner') } - let!(:unused_project_runner) { create(:ci_runner) } - - let!(:project_runner) do - create(:ci_runner, description: 'Project runner').tap do |runner| - create(:ci_runner_project, runner: runner, project: project) - end - end - - let!(:two_projects_runner) do - create(:ci_runner, description: 'Two projects runner').tap do |runner| - create(:ci_runner_project, runner: runner, project: project) - create(:ci_runner_project, runner: runner, project: project2) - end - end - - let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group], runner_type: :group_type) } + let!(:shared_runner) { create(:ci_runner, :instance, description: 'Shared runner') } + let!(:project_runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } + let!(:two_projects_runner) { create(:ci_runner, :project, description: 'Two projects runner', projects: [project, project2]) } + let!(:group_runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } before do # Set project access for users @@ -141,6 +128,18 @@ describe API::Runners do end context 'when runner is not shared' do + context 'when unused runner is present' do + let!(:unused_project_runner) { create(:ci_runner, :project, :without_projects) } + + it 'deletes unused runner' do + expect do + delete api("/runners/#{unused_project_runner.id}", admin) + + expect(response).to have_gitlab_http_status(204) + end.to change { Ci::Runner.specific.count }.by(-1) + end + end + it "returns runner's details" do get api("/runners/#{project_runner.id}", admin) @@ -310,14 +309,6 @@ describe API::Runners do end context 'when runner is not shared' do - it 'deletes unused runner' do - expect do - delete api("/runners/#{unused_project_runner.id}", admin) - - expect(response).to have_gitlab_http_status(204) - end.to change { Ci::Runner.specific.count }.by(-1) - end - it 'deletes used project runner' do expect do delete api("/runners/#{project_runner.id}", admin) @@ -543,11 +534,7 @@ describe API::Runners do describe 'POST /projects/:id/runners' do context 'authorized user' do - let(:project_runner2) do - create(:ci_runner).tap do |runner| - create(:ci_runner_project, runner: runner, project: project2) - end - end + let(:project_runner2) { create(:ci_runner, :project, projects: [project2]) } it 'enables specific runner' do expect do @@ -560,7 +547,7 @@ describe API::Runners do expect do post api("/projects/#{project.id}/runners", user), runner_id: project_runner.id end.to change { project.runners.count }.by(0) - expect(response).to have_gitlab_http_status(409) + expect(response).to have_gitlab_http_status(400) end it 'does not enable locked runner' do @@ -586,11 +573,15 @@ describe API::Runners do end context 'user is admin' do - it 'enables any specific runner' do - expect do - post api("/projects/#{project.id}/runners", admin), runner_id: unused_project_runner.id - end.to change { project.runners.count }.by(+1) - expect(response).to have_gitlab_http_status(201) + context 'when project runner is used' do + let!(:new_project_runner) { create(:ci_runner, :project) } + + it 'enables any specific runner' do + expect do + post api("/projects/#{project.id}/runners", admin), runner_id: new_project_runner.id + end.to change { project.runners.count }.by(+1) + expect(response).to have_gitlab_http_status(201) + end end it 'enables a shared runner' do @@ -603,14 +594,6 @@ describe API::Runners do end end - context 'user is not admin' do - it 'does not enable runner without access to' do - post api("/projects/#{project.id}/runners", user), runner_id: unused_project_runner.id - - expect(response).to have_gitlab_http_status(403) - end - end - it 'raises an error when no runner_id param is provided' do post api("/projects/#{project.id}/runners", admin) @@ -618,6 +601,16 @@ describe API::Runners do end end + context 'user is not admin' do + let!(:new_project_runner) { create(:ci_runner, :project) } + + it 'does not enable runner without access to' do + post api("/projects/#{project.id}/runners", user), runner_id: new_project_runner.id + + expect(response).to have_gitlab_http_status(403) + end + end + context 'authorized user without permissions' do it 'does not enable runner' do post api("/projects/#{project.id}/runners", user2) diff --git a/spec/serializers/runner_entity_spec.rb b/spec/serializers/runner_entity_spec.rb index 439ba2cbca2..ba99d568eba 100644 --- a/spec/serializers/runner_entity_spec.rb +++ b/spec/serializers/runner_entity_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' describe RunnerEntity do - let(:runner) { create(:ci_runner, :specific) } + let(:project) { create(:project) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:entity) { described_class.new(runner, request: request, current_user: user) } let(:request) { double('request') } - let(:project) { create(:project) } let(:user) { create(:admin) } before do diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 8063bc7e1ac..3816bd0deb5 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -5,15 +5,11 @@ module Ci set(:group) { create(:group) } set(:project) { create(:project, group: group, shared_runners_enabled: false, group_runners_enabled: false) } set(:pipeline) { create(:ci_pipeline, project: project) } - let!(:shared_runner) { create(:ci_runner, is_shared: true) } - let!(:specific_runner) { create(:ci_runner, is_shared: false) } - let!(:group_runner) { create(:ci_runner, groups: [group], runner_type: :group_type) } + let!(:shared_runner) { create(:ci_runner, :instance) } + let!(:specific_runner) { create(:ci_runner, :project, projects: [project]) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:pending_job) { create(:ci_build, pipeline: pipeline) } - before do - specific_runner.assign_to(project) - end - describe '#execute' do context 'runner follow tag list' do it "picks build with the same tag" do @@ -181,24 +177,24 @@ module Ci end context 'for multiple builds' do - let!(:project2) { create :project, group_runners_enabled: true, group: group } - let!(:pipeline2) { create :ci_pipeline, project: project2 } - let!(:project3) { create :project, group_runners_enabled: true, group: group } - let!(:pipeline3) { create :ci_pipeline, project: project3 } + let!(:project2) { create(:project, group_runners_enabled: true, group: group) } + let!(:pipeline2) { create(:ci_pipeline, project: project2) } + let!(:project3) { create(:project, group_runners_enabled: true, group: group) } + let!(:pipeline3) { create(:ci_pipeline, project: project3) } let!(:build1_project1) { pending_job } - let!(:build2_project1) { create :ci_build, pipeline: pipeline } - let!(:build3_project1) { create :ci_build, pipeline: pipeline } - let!(:build1_project2) { create :ci_build, pipeline: pipeline2 } - let!(:build2_project2) { create :ci_build, pipeline: pipeline2 } - let!(:build1_project3) { create :ci_build, pipeline: pipeline3 } + let!(:build2_project1) { create(:ci_build, pipeline: pipeline) } + let!(:build3_project1) { create(:ci_build, pipeline: pipeline) } + let!(:build1_project2) { create(:ci_build, pipeline: pipeline2) } + let!(:build2_project2) { create(:ci_build, pipeline: pipeline2) } + let!(:build1_project3) { create(:ci_build, pipeline: pipeline3) } # these shouldn't influence the scheduling - let!(:unrelated_group) { create :group } - let!(:unrelated_project) { create :project, group_runners_enabled: true, group: unrelated_group } - let!(:unrelated_pipeline) { create :ci_pipeline, project: unrelated_project } - let!(:build1_unrelated_project) { create :ci_build, pipeline: unrelated_pipeline } - let!(:unrelated_group_runner) { create :ci_runner, groups: [unrelated_group] } + let!(:unrelated_group) { create(:group) } + let!(:unrelated_project) { create(:project, group_runners_enabled: true, group: unrelated_group) } + let!(:unrelated_pipeline) { create(:ci_pipeline, project: unrelated_project) } + let!(:build1_unrelated_project) { create(:ci_build, pipeline: unrelated_pipeline) } + let!(:unrelated_group_runner) { create(:ci_runner, :group, groups: [unrelated_group]) } it 'does not consider builds from other group runners' do expect(described_class.new(group_runner).send(:builds_for_group_runner).count).to eq 6 @@ -292,7 +288,7 @@ module Ci end context 'when access_level of runner is not_protected' do - let!(:specific_runner) { create(:ci_runner, :specific) } + let!(:specific_runner) { create(:ci_runner, :project, projects: [project]) } context 'when a job is protected' do let!(:pending_job) { create(:ci_build, :protected, pipeline: pipeline) } @@ -324,7 +320,7 @@ module Ci end context 'when access_level of runner is ref_protected' do - let!(:specific_runner) { create(:ci_runner, :ref_protected, :specific) } + let!(:specific_runner) { create(:ci_runner, :project, :ref_protected, projects: [project]) } context 'when a job is protected' do let!(:pending_job) { create(:ci_build, :protected, pipeline: pipeline) } diff --git a/spec/services/ci/update_build_queue_service_spec.rb b/spec/services/ci/update_build_queue_service_spec.rb index 74a23ed2a3f..ca0c6be5da6 100644 --- a/spec/services/ci/update_build_queue_service_spec.rb +++ b/spec/services/ci/update_build_queue_service_spec.rb @@ -6,19 +6,18 @@ describe Ci::UpdateBuildQueueService do let(:pipeline) { create(:ci_pipeline, project: project) } context 'when updating specific runners' do - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } context 'when there is a runner that can pick build' do - before do - build.project.runners << runner - end - it 'ticks runner queue value' do expect { subject.execute(build) }.to change { runner.ensure_runner_queue_value } end end context 'when there is no runner that can pick build' do + let(:another_project) { create(:project) } + let(:runner) { create(:ci_runner, :project, projects: [another_project]) } + it 'does not tick runner queue value' do expect { subject.execute(build) }.not_to change { runner.ensure_runner_queue_value } end @@ -26,7 +25,7 @@ describe Ci::UpdateBuildQueueService do end context 'when updating shared runners' do - let(:runner) { create(:ci_runner, :shared) } + let(:runner) { create(:ci_runner, :instance) } context 'when there is no runner that can pick build' do it 'ticks runner queue value' do @@ -56,9 +55,9 @@ describe Ci::UpdateBuildQueueService do end context 'when updating group runners' do - let(:group) { create :group } - let(:project) { create :project, group: group } - let(:runner) { create :ci_runner, groups: [group] } + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } context 'when there is a runner that can pick build' do it 'ticks runner queue value' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 23b1134b5a3..158541d36e3 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -337,12 +337,18 @@ describe Issues::UpdateService, :mailer do context 'when the labels change' do before do - update_issue(label_ids: [label.id]) + Timecop.freeze(1.minute.from_now) do + update_issue(label_ids: [label.id]) + end end it 'marks todos as done' do expect(todo.reload.done?).to eq true end + + it 'updates updated_at' do + expect(issue.reload.updated_at).to be > Time.now + end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 5279ea6164e..bd2e91f1f7a 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -326,12 +326,18 @@ describe MergeRequests::UpdateService, :mailer do context 'when the labels change' do before do - update_merge_request({ label_ids: [label.id] }) + Timecop.freeze(1.minute.from_now) do + update_merge_request({ label_ids: [label.id] }) + end end it 'marks pending todos as done' do expect(pending_todo.reload).to be_done end + + it 'updates updated_at' do + expect(merge_request.reload.updated_at).to be > Time.now + end end context 'when the assignee changes' do diff --git a/spec/support/gitaly.rb b/spec/support/gitaly.rb index 9cf541372b5..5a1dd44bc9d 100644 --- a/spec/support/gitaly.rb +++ b/spec/support/gitaly.rb @@ -7,7 +7,10 @@ RSpec.configure do |config| next if example.metadata[:skip_gitaly_mock] # Use 'and_wrap_original' to make sure the arguments are valid - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_wrap_original { |m, *args| m.call(*args) || true } + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_wrap_original do |m, *args| + m.call(*args) + !Gitlab::GitalyClient::EXPLICIT_OPT_IN_REQUIRED.include?(args.first) + end end end end diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index 55359d36597..06a76d53354 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/spec/support/helpers/cycle_analytics_helpers.rb @@ -4,12 +4,12 @@ module CycleAnalyticsHelpers create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) end - def create_commit(message, project, user, branch_name, count: 1) + def create_commit(message, project, user, branch_name, count: 1, commit_time: nil, skip_push_handler: false) repository = project.repository - oldrev = repository.commit(branch_name).sha + oldrev = repository.commit(branch_name)&.sha || Gitlab::Git::BLANK_SHA if Timecop.frozen? && Gitlab::GitalyClient.feature_enabled?(:operation_user_commit_files) - mock_gitaly_multi_action_dates(repository.raw) + mock_gitaly_multi_action_dates(repository.raw, commit_time) end commit_shas = Array.new(count) do |index| @@ -19,6 +19,8 @@ module CycleAnalyticsHelpers commit_sha end + return if skip_push_handler + GitPushService.new(project, user, oldrev: oldrev, @@ -44,13 +46,11 @@ module CycleAnalyticsHelpers project.repository.add_branch(user, source_branch, 'master') end - sha = project.repository.create_file( - user, - generate(:branch), - 'content', - message: commit_message, - branch_name: source_branch) - project.repository.commit(sha) + # Cycle analytic specs often test with frozen times, which causes metrics to be + # pinned to the current time. For example, in the plan stage, we assume that an issue + # milestone has been created before any code has been written. We add a second + # to ensure that the plan time is positive. + create_commit(commit_message, project, user, source_branch, commit_time: Time.now + 1.second, skip_push_handler: true) opts = { title: 'Awesome merge_request', @@ -116,9 +116,9 @@ module CycleAnalyticsHelpers protected: false) end - def mock_gitaly_multi_action_dates(raw_repository) + def mock_gitaly_multi_action_dates(raw_repository, commit_time) allow(raw_repository).to receive(:multi_action).and_wrap_original do |m, *args| - new_date = Time.now + new_date = commit_time || Time.now branch_update = m.call(*args) if branch_update.newrev diff --git a/spec/support/shared_examples/file_finder.rb b/spec/support/shared_examples/file_finder.rb new file mode 100644 index 00000000000..ef144bdf61c --- /dev/null +++ b/spec/support/shared_examples/file_finder.rb @@ -0,0 +1,21 @@ +shared_examples 'file finder' do + let(:query) { 'files' } + let(:search_results) { subject.find(query) } + + it 'finds by name' do + filename, blob = search_results.find { |_, blob| blob.filename == expected_file_by_name } + expect(filename).to eq(expected_file_by_name) + expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) + expect(blob.ref).to eq(subject.ref) + expect(blob.data).not_to be_empty + end + + it 'finds by content' do + filename, blob = search_results.find { |_, blob| blob.filename == expected_file_by_content } + + expect(filename).to eq(expected_file_by_content) + expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) + expect(blob.ref).to eq(subject.ref) + expect(blob.data).not_to be_empty + end +end diff --git a/spec/support/shared_examples/models/atomic_internal_id_spec.rb b/spec/support/shared_examples/models/atomic_internal_id_spec.rb index 6a6e13418a9..7ab1041d17c 100644 --- a/spec/support/shared_examples/models/atomic_internal_id_spec.rb +++ b/spec/support/shared_examples/models/atomic_internal_id_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -shared_examples_for 'AtomicInternalId' do +shared_examples_for 'AtomicInternalId' do |validate_presence: true| describe '.has_internal_id' do describe 'Module inclusion' do subject { described_class } @@ -9,14 +9,31 @@ shared_examples_for 'AtomicInternalId' do end describe 'Validation' do - subject { instance } - before do - allow(InternalId).to receive(:generate_next).and_return(nil) + allow_any_instance_of(described_class).to receive(:"ensure_#{scope}_#{internal_id_attribute}!") + + instance.valid? end - it { is_expected.to validate_presence_of(internal_id_attribute) } - it { is_expected.to validate_numericality_of(internal_id_attribute) } + context 'when presence validation is required' do + before do + skip unless validate_presence + end + + it 'validates presence' do + expect(instance.errors[internal_id_attribute]).to include("can't be blank") + end + end + + context 'when presence validation is not required' do + before do + skip if validate_presence + end + + it 'does not validate presence' do + expect(instance.errors[internal_id_attribute]).to be_empty + end + end end describe 'Creating an instance' do diff --git a/spec/support/shared_examples/url_validator_examples.rb b/spec/support/shared_examples/url_validator_examples.rb new file mode 100644 index 00000000000..b4757a70984 --- /dev/null +++ b/spec/support/shared_examples/url_validator_examples.rb @@ -0,0 +1,42 @@ +RSpec.shared_examples 'url validator examples' do |protocols| + let(:validator) { described_class.new(attributes: [:link_url], **options) } + let!(:badge) { build(:badge, link_url: 'http://www.example.com') } + + subject { validator.validate_each(badge, :link_url, badge.link_url) } + + describe '#validates_each' do + context 'with no options' do + let(:options) { {} } + + it "allows #{protocols.join(',')} protocols by default" do + expect(validator.send(:default_options)[:protocols]).to eq protocols + end + + it 'checks that the url structure is valid' do + badge.link_url = "#{badge.link_url}:invalid_port" + + subject + + expect(badge.errors.empty?).to be false + end + end + + context 'with protocols' do + let(:options) { { protocols: %w[http] } } + + it 'allows urls with the defined protocols' do + subject + + expect(badge.errors.empty?).to be true + end + + it 'add error if the url protocol does not match the selected ones' do + badge.link_url = 'https://www.example.com' + + subject + + expect(badge.errors.empty?).to be false + end + end + end +end diff --git a/spec/validators/public_url_validator_spec.rb b/spec/validators/public_url_validator_spec.rb new file mode 100644 index 00000000000..710dd3dc38e --- /dev/null +++ b/spec/validators/public_url_validator_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe PublicUrlValidator do + include_examples 'url validator examples', described_class::DEFAULT_PROTOCOLS + + context 'by default' do + let(:validator) { described_class.new(attributes: [:link_url]) } + let!(:badge) { build(:badge, link_url: 'http://www.example.com') } + + subject { validator.validate_each(badge, :link_url, badge.link_url) } + + it 'blocks urls pointing to localhost' do + badge.link_url = 'https://127.0.0.1' + + subject + + expect(badge.errors.empty?).to be false + end + + it 'blocks urls pointing to the local network' do + badge.link_url = 'https://192.168.1.1' + + subject + + expect(badge.errors.empty?).to be false + end + end +end diff --git a/spec/validators/url_placeholder_validator_spec.rb b/spec/validators/url_placeholder_validator_spec.rb deleted file mode 100644 index b76d8acdf88..00000000000 --- a/spec/validators/url_placeholder_validator_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -require 'spec_helper' - -describe UrlPlaceholderValidator do - let(:validator) { described_class.new(attributes: [:link_url], **options) } - let!(:badge) { build(:badge) } - let(:placeholder_url) { 'http://www.example.com/%{project_path}/%{project_id}/%{default_branch}/%{commit_sha}' } - - subject { validator.validate_each(badge, :link_url, badge.link_url) } - - describe '#validates_each' do - context 'with no options' do - let(:options) { {} } - - it 'allows http and https protocols by default' do - expect(validator.send(:default_options)[:protocols]).to eq %w(http https) - end - - it 'checks that the url structure is valid' do - badge.link_url = placeholder_url - - subject - - expect(badge.errors.empty?).to be false - end - end - - context 'with placeholder regex' do - let(:options) { { placeholder_regex: /(project_path|project_id|commit_sha|default_branch)/ } } - - it 'checks that the url is valid and obviate placeholders that match regex' do - badge.link_url = placeholder_url - - subject - - expect(badge.errors.empty?).to be true - end - end - end -end diff --git a/spec/validators/url_validator_spec.rb b/spec/validators/url_validator_spec.rb index 763dff181d2..2d719263fc8 100644 --- a/spec/validators/url_validator_spec.rb +++ b/spec/validators/url_validator_spec.rb @@ -1,46 +1,62 @@ require 'spec_helper' describe UrlValidator do - let(:validator) { described_class.new(attributes: [:link_url], **options) } - let!(:badge) { build(:badge) } - + let!(:badge) { build(:badge, link_url: 'http://www.example.com') } subject { validator.validate_each(badge, :link_url, badge.link_url) } - describe '#validates_each' do - context 'with no options' do - let(:options) { {} } + include_examples 'url validator examples', described_class::DEFAULT_PROTOCOLS + + context 'by default' do + let(:validator) { described_class.new(attributes: [:link_url]) } + + it 'does not block urls pointing to localhost' do + badge.link_url = 'https://127.0.0.1' + + subject + + expect(badge.errors.empty?).to be true + end + + it 'does not block urls pointing to the local network' do + badge.link_url = 'https://192.168.1.1' - it 'allows http and https protocols by default' do - expect(validator.send(:default_options)[:protocols]).to eq %w(http https) - end + subject - it 'checks that the url structure is valid' do - badge.link_url = 'http://www.google.es/%{whatever}' + expect(badge.errors.empty?).to be true + end + end + + context 'when allow_localhost is set to false' do + let(:validator) { described_class.new(attributes: [:link_url], allow_localhost: false) } + + it 'blocks urls pointing to localhost' do + badge.link_url = 'https://127.0.0.1' - subject + subject - expect(badge.errors.empty?).to be false - end + expect(badge.errors.empty?).to be false end + end - context 'with protocols' do - let(:options) { { protocols: %w(http) } } + context 'when allow_local_network is set to false' do + let(:validator) { described_class.new(attributes: [:link_url], allow_local_network: false) } - it 'allows urls with the defined protocols' do - badge.link_url = 'http://www.example.com' + it 'blocks urls pointing to the local network' do + badge.link_url = 'https://192.168.1.1' - subject + subject - expect(badge.errors.empty?).to be true - end + expect(badge.errors.empty?).to be false + end + end - it 'add error if the url protocol does not match the selected ones' do - badge.link_url = 'https://www.example.com' + context 'when ports is set' do + let(:validator) { described_class.new(attributes: [:link_url], ports: [443]) } - subject + it 'blocks urls with a different port' do + subject - expect(badge.errors.empty?).to be false - end + expect(badge.errors.empty?).to be false end end end |