diff options
87 files changed, 1043 insertions, 259 deletions
diff --git a/.gitlab/issue_templates/Security developer workflow.md b/.gitlab/issue_templates/Security developer workflow.md index 4bc4215d21b..aaa16145399 100644 --- a/.gitlab/issue_templates/Security developer workflow.md +++ b/.gitlab/issue_templates/Security developer workflow.md @@ -20,10 +20,9 @@ Set the title to: `Description of the original issue` - [ ] Once the MR is ready to be merged, create MRs targetting the last 3 releases, plus the current RC if between the 7th and 22nd of the month. - [ ] At this point, it might be easy to squash the commits from the MR into one - You can use the script `bin/secpick` instead of the following steps, to help you cherry-picking. See the [secpick documentation] - - [ ] Create the branch `security-X-Y` from `X-Y-stable` if it doesn't exist (and make sure it's up to date with stable) - - [ ] Create each MR targetting the security branch `security-X-Y` - - [ ] Add the ~security label and prefix with the version `WIP: [X.Y]` the title of the MR -- [ ] Add the ~"Merge into Security" label to all of the MRs. + - [ ] Create each MR targetting the stable branch `X-Y-stable`, using the "Security Release" merge request template. + - Every merge request will have its own set of TODOs, so make sure to + complete those. - [ ] Make sure all MRs have a link in the [links section](#links) [secpick documentation]: https://gitlab.com/gitlab-org/release/docs/blob/master/general/security/developer.md#secpick-script diff --git a/.gitlab/merge_request_templates/Security Release.md b/.gitlab/merge_request_templates/Security Release.md index 9a0979f27a7..246f2dae009 100644 --- a/.gitlab/merge_request_templates/Security Release.md +++ b/.gitlab/merge_request_templates/Security Release.md @@ -4,6 +4,9 @@ This MR should be created on `dev.gitlab.org`. See [the general developer security release guidelines](https://gitlab.com/gitlab-org/release/docs/blob/master/general/security/developer.md). +This merge request _must not_ close the corresponding security issue _unless_ it +targets master. + --> ## Related issues @@ -12,7 +15,7 @@ See [the general developer security release guidelines](https://gitlab.com/gitla ## Developer checklist - [ ] Link to the developer security workflow issue on `dev.gitlab.org` -- [ ] MR targets `master` or `security-X-Y` for backports +- [ ] MR targets `master`, or `X-Y-stable` for backports - [ ] Milestone is set for the version this MR applies to - [ ] Title of this MR is the same as for all backports - [ ] A [CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html) is added without a `merge_request` value, with `type` set to `security` @@ -25,4 +28,4 @@ See [the general developer security release guidelines](https://gitlab.com/gitla - [ ] Correct milestone is applied and the title is matching across all backports - [ ] Assigned to `@gitlab-release-tools-bot` with passing CI pipelines -/label ~security ~"Merge into Security" +/label ~security diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 092afa15df4..744068368fb 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.17.0 +1.18.0
\ No newline at end of file diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index fbb9ea12de3..2bf50aaf17a 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.2.0 +8.3.0 @@ -422,7 +422,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 1.5.0', require: 'gitaly' +gem 'gitaly-proto', '~> 1.10.0', require: 'gitaly' gem 'grpc', '~> 1.15.0' gem 'google-protobuf', '~> 3.6' diff --git a/Gemfile.lock b/Gemfile.lock index f661da41507..0b2bd2c96bd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -278,7 +278,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (1.5.0) + gitaly-proto (1.10.0) grpc (~> 1.0) github-markup (1.7.0) gitlab-default_value_for (3.1.1) @@ -1020,7 +1020,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 1.5.0) + gitaly-proto (~> 1.10.0) github-markup (~> 1.7.0) gitlab-default_value_for (~> 3.1.1) gitlab-markup (~> 1.6.5) diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 037a5adfb7e..4d8f1ee51a6 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -4,6 +4,7 @@ */ .file-holder { border: 1px solid $border-color; + border-top: 0; border-radius: $border-radius-default; &.file-holder-no-border { @@ -328,6 +329,7 @@ span.idiff { justify-content: space-between; background-color: $gray-light; border-bottom: 1px solid $border-color; + border-top: 1px solid $border-color; padding: 5px $gl-padding; margin: 0; border-radius: $border-radius-default $border-radius-default 0 0; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 9eae9a831fa..7088a6f59dc 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -490,6 +490,7 @@ $builds-trace-bg: #111; */ $commit-max-width-marker-color: rgba(0, 0, 0, 0); $commit-message-text-area-bg: rgba(0, 0, 0, 0); +$commit-stat-summary-height: 36px; /* * Common @@ -664,8 +665,14 @@ $priority-label-empty-state-width: 114px; Issues Analytics */ $issues-analytics-popover-boarder-color: rgba(0, 0, 0, 0.15); + /* Merge Requests */ $mr-tabs-height: 51px; $mr-version-controls-height: 56px; + +/* +Compare Branches +*/ +$compare-branches-sticky-header-height: 68px; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 02aac58a475..a225645c643 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -7,22 +7,13 @@ cursor: pointer; @media (min-width: map-get($grid-breakpoints, md)) { + $mr-file-header-top: $mr-version-controls-height + $header-height + $mr-tabs-height; + position: -webkit-sticky; position: sticky; - top: $mr-version-controls-height + $header-height + $mr-tabs-height; - margin-left: -1px; - border-left: 1px solid $border-color; + top: $mr-file-header-top; z-index: 102; - &.is-commit { - top: $header-height + 36px; - - .with-performance-bar & { - top: $header-height + 36px + $performance-bar-height; - - } - } - &::before { content: ''; position: absolute; @@ -35,7 +26,23 @@ } .with-performance-bar & { - top: $header-height + $performance-bar-height + $mr-version-controls-height + $mr-tabs-height; + top: $mr-file-header-top + $performance-bar-height; + } + + &.is-commit { + top: $header-height + $commit-stat-summary-height; + + .with-performance-bar & { + top: $header-height + $commit-stat-summary-height + $performance-bar-height; + } + } + + &.is-compare { + top: $header-height + $compare-branches-sticky-header-height; + + .with-performance-bar & { + top: $performance-bar-height + $header-height + $compare-branches-sticky-header-height; + } } } diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb index 37ac11dc6a1..a27e3cceaeb 100644 --- a/app/controllers/profiles/preferences_controller.rb +++ b/app/controllers/profiles/preferences_controller.rb @@ -33,12 +33,10 @@ class Profiles::PreferencesController < Profiles::ApplicationController end def preferences_params - params.require(:user).permit( - :color_scheme_id, - :layout, - :dashboard, - :project_view, - :theme_id - ) + params.require(:user).permit(preferences_param_names) + end + + def preferences_param_names + [:color_scheme_id, :layout, :dashboard, :project_view, :theme_id] end end diff --git a/app/controllers/projects/error_tracking_controller.rb b/app/controllers/projects/error_tracking_controller.rb index 9e403e1d25b..88d0755f41f 100644 --- a/app/controllers/projects/error_tracking_controller.rb +++ b/app/controllers/projects/error_tracking_controller.rb @@ -15,6 +15,14 @@ class Projects::ErrorTrackingController < Projects::ApplicationController end end + def list_projects + respond_to do |format| + format.json do + render_project_list_json + end + end + end + private def render_index_json @@ -32,6 +40,32 @@ class Projects::ErrorTrackingController < Projects::ApplicationController } end + def render_project_list_json + service = ErrorTracking::ListProjectsService.new( + project, + current_user, + list_projects_params + ) + result = service.execute + + if result[:status] == :success + render json: { + projects: serialize_projects(result[:projects]) + } + else + return render( + status: result[:http_status] || :bad_request, + json: { + message: result[:message] + } + ) + end + end + + def list_projects_params + params.require(:error_tracking_setting).permit([:api_host, :token]) + end + def set_polling_interval Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL) end @@ -41,4 +75,10 @@ class Projects::ErrorTrackingController < Projects::ApplicationController .new(project: project, user: current_user) .represent(errors) end + + def serialize_projects(projects) + ErrorTracking::ProjectSerializer + .new(project: project, user: current_user) + .represent(projects) + end end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index c4e310e638d..a3d662d8250 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -7,6 +7,12 @@ class ApplicationRecord < ActiveRecord::Base where(id: ids) end + def self.safe_find_or_create_by!(*args) + safe_find_or_create_by(*args).tap do |record| + record.validate! unless record.persisted? + end + end + def self.safe_find_or_create_by(*args) transaction(requires_new: true) do find_or_create_by(*args) diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index 0816778deae..7f9ff7bbda6 100644 --- a/app/models/gpg_signature.rb +++ b/app/models/gpg_signature.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class GpgSignature < ActiveRecord::Base +class GpgSignature < ApplicationRecord include ShaAttribute sha_attribute :commit_sha @@ -33,6 +33,11 @@ class GpgSignature < ActiveRecord::Base ) end + def self.safe_create!(attributes) + create_with(attributes) + .safe_find_or_create_by!(commit_sha: attributes[:commit_sha]) + end + def gpg_key=(model) case model when GpgKey diff --git a/app/models/pool_repository.rb b/app/models/pool_repository.rb index 34220c1b450..4635fc72dc7 100644 --- a/app/models/pool_repository.rb +++ b/app/models/pool_repository.rb @@ -96,7 +96,9 @@ class PoolRepository < ActiveRecord::Base @object_pool ||= Gitlab::Git::ObjectPool.new( shard.name, disk_path + '.git', - source_project.repository.raw) + source_project.repository.raw, + source_project.full_path + ) end def inspect diff --git a/app/models/project.rb b/app/models/project.rb index d4e2ed883bc..8f746f6e094 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1288,7 +1288,7 @@ class Project < ActiveRecord::Base # Forked import is handled asynchronously return if forked? && !force - if gitlab_shell.create_repository(repository_storage, disk_path) + if gitlab_shell.create_project_repository(self) repository.after_create true else diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 559e4f99294..c43bd45a62f 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -60,7 +60,7 @@ class ProjectWiki def wiki @wiki ||= begin gl_repository = Gitlab::GlRepository.gl_repository(project, true) - raw_repository = Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', gl_repository) + raw_repository = Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', gl_repository, full_path) create_repo!(raw_repository) unless raw_repository.exists? @@ -175,7 +175,7 @@ class ProjectWiki private def create_repo!(raw_repository) - gitlab_shell.create_repository(project.repository_storage, disk_path) + gitlab_shell.create_wiki_repository(project) raise CouldNotCreateWikiError unless raw_repository.exists? diff --git a/app/models/repository.rb b/app/models/repository.rb index bfd2608bed4..7c50b4488e5 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1104,6 +1104,9 @@ class Repository end def initialize_raw_repository - Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, is_wiki)) + Gitlab::Git::Repository.new(project.repository_storage, + disk_path + '.git', + Gitlab::GlRepository.gl_repository(project, is_wiki), + project.full_path) end end diff --git a/app/serializers/error_tracking/project_serializer.rb b/app/serializers/error_tracking/project_serializer.rb index b2406f4d631..68724088fff 100644 --- a/app/serializers/error_tracking/project_serializer.rb +++ b/app/serializers/error_tracking/project_serializer.rb @@ -2,6 +2,6 @@ module ErrorTracking class ProjectSerializer < BaseSerializer - entity ProjectEntity + entity ErrorTracking::ProjectEntity end end diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb new file mode 100644 index 00000000000..c6e8be0f2be --- /dev/null +++ b/app/services/error_tracking/list_projects_service.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module ErrorTracking + class ListProjectsService < ::BaseService + def execute + return error('access denied') unless can_read? + + setting = project_error_tracking_setting + + unless setting.valid? + return error(setting.errors.full_messages.join(', '), :bad_request) + end + + begin + result = setting.list_sentry_projects + rescue Sentry::Client::Error => e + return error(e.message, :bad_request) + rescue Sentry::Client::SentryError => e + return error(e.message, :unprocessable_entity) + end + + success(projects: result[:projects]) + end + + private + + def project_error_tracking_setting + (project.error_tracking_setting || project.build_error_tracking_setting).tap do |setting| + setting.api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from( + api_host: params[:api_host], + organization_slug: nil, + project_slug: nil + ) + + setting.token = params[:token] + setting.enabled = true + end + end + + def can_read? + can?(current_user, :read_sentry_issue, project) + end + end +end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 5861b803996..7214e9efaf6 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -73,7 +73,7 @@ module Projects project.ensure_repository project.repository.fetch_as_mirror(project.import_url, refmap: refmap) else - gitlab_shell.import_repository(project.repository_storage, project.disk_path, project.import_url) + gitlab_shell.import_project_repository(project) end rescue Gitlab::Shell::Error => e # Expire cache to prevent scenarios such as: diff --git a/app/views/layouts/nav/sidebar/_group.html.haml b/app/views/layouts/nav/sidebar/_group.html.haml index bf475c07711..3fbaaafe89e 100644 --- a/app/views/layouts/nav/sidebar/_group.html.haml +++ b/app/views/layouts/nav/sidebar/_group.html.haml @@ -36,7 +36,7 @@ %span = _('Activity') - = render_if_exists 'groups/sidebar/security_dashboard' + = render_if_exists 'groups/sidebar/security_dashboard' # EE-specific - if group_sidebar_link?(:contribution_analytics) = nav_link(path: 'analytics#show') do diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 7c378633667..c1616810185 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -51,6 +51,9 @@ = f.label :dashboard, class: 'label-bold' do Default dashboard = f.select :dashboard, dashboard_choices, {}, class: 'form-control' + + = render_if_exists 'profiles/preferences/group_overview_selector', f: f # EE-specific + .form-group = f.label :project_view, class: 'label-bold' do Project overview content diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index 06f0cd9675e..fe9a8ac4182 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -10,7 +10,7 @@ .container-fluid{ class: [limited_container_width, container_class] } = render "commit_box" = render "ci_menu" - = render "projects/diffs/diffs", diffs: @diffs, environment: @environment, is_commit: true + = render "projects/diffs/diffs", diffs: @diffs, environment: @environment, diff_page_context: "is-commit" .limited-width-notes = render "shared/notes/notes_with_form", :autocomplete => true diff --git a/app/views/projects/compare/show.html.haml b/app/views/projects/compare/show.html.haml index b6bebbabed0..5774b48a054 100644 --- a/app/views/projects/compare/show.html.haml +++ b/app/views/projects/compare/show.html.haml @@ -8,7 +8,7 @@ - if @commits.present? = render "projects/commits/commit_list" - = render "projects/diffs/diffs", diffs: @diffs, environment: @environment + = render "projects/diffs/diffs", diffs: @diffs, environment: @environment, diff_page_context: "is-compare" - else .card.bg-light .center diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index cc2d0d3b2d8..2dba3fcd664 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -2,7 +2,7 @@ - show_whitespace_toggle = local_assigns.fetch(:show_whitespace_toggle, true) - can_create_note = !@diff_notes_disabled && can?(current_user, :create_note, diffs.project) - diff_files = diffs.diff_files -- is_commit = local_assigns.fetch(:is_commit, false) +- diff_page_context = local_assigns.fetch(:diff_page_context, nil) .content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed .files-changed-inner @@ -25,4 +25,4 @@ = render 'projects/diffs/warning', diff_files: diffs .files{ data: { can_create_note: can_create_note } } - = render partial: 'projects/diffs/file', collection: diff_files, as: :diff_file, locals: { project: diffs.project, environment: environment, is_commit: is_commit } + = render partial: 'projects/diffs/file', collection: diff_files, as: :diff_file, locals: { project: diffs.project, environment: environment, diff_page_context: diff_page_context } diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 5565ae1d98b..855b719dc45 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -1,11 +1,11 @@ - environment = local_assigns.fetch(:environment, nil) -- is_commit = local_assigns.fetch(:is_commit, false) +- diff_page_context = local_assigns.fetch(:diff_page_context, nil) - file_hash = hexdigest(diff_file.file_path) - image_diff = diff_file.rich_viewer && diff_file.rich_viewer.partial_name == 'image' - image_replaced = diff_file.old_content_sha && diff_file.old_content_sha != diff_file.content_sha .diff-file.file-holder{ id: file_hash, data: diff_file_html_data(project, diff_file.file_path, diff_file.content_sha) } - .js-file-title.file-title-flex-parent{ class: is_commit ? "is-commit" : "" } + .js-file-title.file-title-flex-parent{ class: diff_page_context } .file-header-content = render "projects/diffs/file_header", diff_file: diff_file, url: "##{file_hash}" diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index 7eae07d3f6b..a9b88a133be 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -15,19 +15,19 @@ class RepositoryForkWorker return target_project.import_state.mark_as_failed(_('Source project cannot be found.')) end - fork_repository(target_project, source_project.repository_storage, source_project.disk_path) + fork_repository(target_project, source_project) end private - def fork_repository(target_project, source_repository_storage_name, source_disk_path) + def fork_repository(target_project, source_project) return unless start_fork(target_project) Gitlab::Metrics.add_event(:fork_repository) - result = gitlab_shell.fork_repository(source_repository_storage_name, source_disk_path, - target_project.repository_storage, target_project.disk_path) - raise "Unable to fork project #{target_project.id} for repository #{source_disk_path} -> #{target_project.disk_path}" unless result + result = gitlab_shell.fork_repository(source_project, target_project) + + raise "Unable to fork project #{target_project.id} for repository #{source_project.disk_path} -> #{target_project.disk_path}" unless result target_project.after_import end diff --git a/bin/secpick b/bin/secpick index be120a304c9..8f956d300a7 100755 --- a/bin/secpick +++ b/bin/secpick @@ -10,6 +10,7 @@ using Rainbow module Secpick BRANCH_PREFIX = 'security'.freeze + STABLE_SUFFIX = 'stable'.freeze DEFAULT_REMOTE = 'dev'.freeze NEW_MR_URL = 'https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/new'.freeze @@ -36,16 +37,16 @@ module Secpick branch.freeze end - def security_branch - "#{BRANCH_PREFIX}-#{@options[:version]}".tap do |name| + def stable_branch + "#{@options[:version]}-#{STABLE_SUFFIX}".tap do |name| name << "-ee" if ee? end.freeze end def git_commands - ["git fetch #{@options[:remote]} #{security_branch}", - "git checkout #{security_branch}", - "git pull #{@options[:remote]} #{security_branch}", + ["git fetch #{@options[:remote]} #{stable_branch}", + "git checkout #{stable_branch}", + "git pull #{@options[:remote]} #{stable_branch}", "git checkout -B #{source_branch}", "git cherry-pick #{@options[:sha]}", "git push #{@options[:remote]} #{source_branch}", @@ -56,9 +57,8 @@ module Secpick { merge_request: { source_branch: source_branch, - target_branch: security_branch, - title: "[#{@options[:version].tr('-', '.')}] ", - description: '/label ~security ~"Merge into Security"' + target_branch: stable_branch, + description: '/label ~security' } } end diff --git a/changelogs/unreleased/44332-add-openid-profile-scopes.yml b/changelogs/unreleased/44332-add-openid-profile-scopes.yml new file mode 100644 index 00000000000..b554fab5139 --- /dev/null +++ b/changelogs/unreleased/44332-add-openid-profile-scopes.yml @@ -0,0 +1,5 @@ +--- +title: GitLab now supports the profile and email scopes from OpenID Connect +merge_request: 24335 +author: Goten Xiao +type: added diff --git a/changelogs/unreleased/56938-diff-file-headers-on-compare-not-quite-right.yml b/changelogs/unreleased/56938-diff-file-headers-on-compare-not-quite-right.yml new file mode 100644 index 00000000000..f619a009a63 --- /dev/null +++ b/changelogs/unreleased/56938-diff-file-headers-on-compare-not-quite-right.yml @@ -0,0 +1,5 @@ +--- +title: Correct spacing for comparison page +merge_request: !24783 +author: +type: fixed diff --git a/changelogs/unreleased/bvl-fix-race-condition-creating-signature.yml b/changelogs/unreleased/bvl-fix-race-condition-creating-signature.yml new file mode 100644 index 00000000000..307b4f526bb --- /dev/null +++ b/changelogs/unreleased/bvl-fix-race-condition-creating-signature.yml @@ -0,0 +1,5 @@ +--- +title: Avoid race conditions when creating GpgSignature +merge_request: 24939 +author: +type: fixed diff --git a/changelogs/unreleased/gitaly-update-1.18.0.yml b/changelogs/unreleased/gitaly-update-1.18.0.yml new file mode 100644 index 00000000000..392527f5e5d --- /dev/null +++ b/changelogs/unreleased/gitaly-update-1.18.0.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade gitaly to 1.18.0 +merge_request: 24981 +author: +type: other diff --git a/changelogs/unreleased/workhorse-8-3-0.yml b/changelogs/unreleased/workhorse-8-3-0.yml new file mode 100644 index 00000000000..6ae01d64ae5 --- /dev/null +++ b/changelogs/unreleased/workhorse-8-3-0.yml @@ -0,0 +1,5 @@ +--- +title: Update Workhorse to v8.3.0 +merge_request: 24959 +author: +type: other diff --git a/config/initializers/doorkeeper_openid_connect.rb b/config/initializers/doorkeeper_openid_connect.rb index e97c0fcbd6b..fd5a62c39c6 100644 --- a/config/initializers/doorkeeper_openid_connect.rb +++ b/config/initializers/doorkeeper_openid_connect.rb @@ -31,8 +31,27 @@ Doorkeeper::OpenidConnect.configure do o.claim(:name) { |user| user.name } o.claim(:nickname) { |user| user.username } - o.claim(:email) { |user| user.public_email } - o.claim(:email_verified) { |user| true if user.public_email? } + + # Check whether the application has access to the email scope, and grant + # access to the user's primary email address if so, otherwise their + # public email address (if present) + # This allows existing solutions built for GitLab's old behavior to keep + # working without modification. + o.claim(:email) do |user, scopes| + scopes.exists?(:email) ? user.email : user.public_email + end + o.claim(:email_verified) do |user, scopes| + if scopes.exists?(:email) + user.primary_email_verified? + elsif user.public_email? + user.verified_email?(user.public_email) + else + # If there is no public email set, tell doorkicker-openid-connect to + # exclude the email_verified claim by returning nil. + nil + end + end + o.claim(:website) { |user| user.full_website_url if user.website_url? } o.claim(:profile) { |user| Gitlab::Routing.url_helpers.user_url user } o.claim(:picture) { |user| user.avatar_url(only_path: false) } diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index 9f451046462..a2dff92908e 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -64,6 +64,8 @@ en: read_registry: Grants permission to read container registry images openid: Authenticate using OpenID Connect sudo: Perform API actions as any user in the system + profile: Allows read-only access to the user's personal information using OpenID Connect + email: Allows read-only access to the user's primary email address using OpenID Connect scope_desc: api: Grants complete read/write access to the API, including all groups and projects. @@ -77,6 +79,10 @@ en: Grants permission to authenticate with GitLab using OpenID Connect. Also gives read-only access to the user's profile and group memberships. sudo: Grants permission to perform API actions as any user in the system, when authenticated as an admin user. + profile: + Grants read-only access to the user's profile data using OpenID Connect. + email: + Grants read-only access to the user's primary email address using OpenID Connect. flash: applications: create: diff --git a/config/routes/project.rb b/config/routes/project.rb index d730479cf2b..b4ebc7df4fe 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -444,7 +444,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do end end - resources :error_tracking, only: [:index], controller: :error_tracking + resources :error_tracking, only: [:index], controller: :error_tracking do + collection do + post :list_projects + end + end # Since both wiki and repository routing contains wildcard characters # its preferable to keep it below all other project routes diff --git a/db/migrate/20140502125220_migrate_repo_size.rb b/db/migrate/20140502125220_migrate_repo_size.rb index a69b02cddc4..bff1f01c654 100644 --- a/db/migrate/20140502125220_migrate_repo_size.rb +++ b/db/migrate/20140502125220_migrate_repo_size.rb @@ -11,7 +11,7 @@ class MigrateRepoSize < ActiveRecord::Migration[4.2] path = File.join(namespace_path, project['project_path'] + '.git') begin - repo = Gitlab::Git::Repository.new('default', path, '') + repo = Gitlab::Git::Repository.new('default', path, '', '') if repo.empty? print '-' else diff --git a/doc/development/README.md b/doc/development/README.md index 05715274a81..d5829e31343 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -123,3 +123,7 @@ description: 'Learn how to contribute to GitLab.' ## Compliance - [Licensing](licensing.md) for ensuring license compliance + +## Go guides + +- [Go Guidelines](go_guide/index.md) diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index 6f1ba5d62a5..0eedef5e14f 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -21,6 +21,7 @@ of _prohibited this user from being saved due to the following errors:_ the text should be _sorry, we could not create your account because:_ 1. Code should be written in [US English][us-english] +1. [Go](../go_guide/index.md) This is also the style used by linting tools such as [RuboCop](https://github.com/bbatsov/rubocop) and [Hound CI](https://houndci.com). diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md new file mode 100644 index 00000000000..cdc806a2d31 --- /dev/null +++ b/doc/development/go_guide/index.md @@ -0,0 +1,216 @@ +# Go standards and style guidelines + +This document describes various guidelines and best practices for GitLab +projects using the [Go language](https://golang.org). + +## Overview + +GitLab is built on top of [Ruby on Rails](https://rubyonrails.org/), but we're +also using Go for projects where it makes sense. Go is a very powerful +language, with many advantages, and is best suited for projects with a lot of +IO (disk/network access), HTTP requests, parallel processing, etc. Since we +have both Ruby on Rails and Go at GitLab, we should evaluate carefully which of +the two is best for the job. + +This page aims to define and organize our Go guidelines, based on our various +experiences. Several projects were started with different standards and they +can still have specifics. They will be described in their respective +`README.md` or `PROCESS.md` files. + +## Code Review + +We follow the common principles of +[Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments). + +Reviewers and maintainers should pay attention to: + +- `defer` functions: ensure the presence when needed, and after `err` check. +- Inject dependencies as parameters. +- Void structs when marshalling to JSON (generates `null` instead of `[]`). + +### Security + +Security is our top priority at GitLab. During code reviews, we must take care +of possible security breaches in our code: + +- XSS when using text/template +- CSRF Protection using Gorilla +- Use a Go version without known vulnerabilities +- Don't leak secret tokens +- SQL injections + +Remember to run +[SAST](https://docs.gitlab.com/ee/user/project/merge_requests/sast.html) +**[ULTIMATE]** on your project (or at least the [gosec +analyzer](https://gitlab.com/gitlab-org/security-products/analyzers/gosec)), +and to follow our [Security +requirements](../code_review.md#security-requirements). + +Web servers can take advantages of middlewares like [Secure](https://github.com/unrolled/secure). + +### Finding a reviewer + +Many of our projects are too small to have full-time maintainers. That's why we +have a shared pool of Go reviewers at GitLab. To find a reviewer, use the +[Engineering Projects](https://about.gitlab.com/handbook/engineering/projects/) +page in the handbook. "GitLab Community Edition (CE)" and "GitLab Community +Edition (EE)" both have a "Go" section with its list of reviewers. + +To add yourself to this list, add the following to your profile in the +[team.yml](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/team.yml) +file and ask your manager to review and merge. + +```yaml +projects: + gitlab-ee: reviewer go + gitlab-ce: reviewer go +``` + +## Code style and format + +- Avoid global variables, even in packages. By doing so you will introduce side + effects if the package is included multiple times. +- Use `go fmt` before committing ([Gofmt](https://golang.org/cmd/gofmt/) is a + tool that automatically formats Go source code). + +### Automatic linting + +All Go projects should include these GitLab CI/CD jobs: + +```yaml +go lint: + image: golang:1.11 + script: + - go get -u golang.org/x/lint/golint + - golint -set_exit_status +``` + +Once [recursive includes](https://gitlab.com/gitlab-org/gitlab-ce/issues/56836) +become available, you will be able to share job templates like this +[analyzer](https://gitlab.com/gitlab-org/security-products/ci-templates/raw/master/includes-dev/analyzer.yml). + +## Dependencies + +Dependencies should be kept to the minimum. The introduction of a new +dependency should be argued in the merge request, as per our [Approval +Guidelines](../code_review.html#approval-guidelines). Both [License +Management](https://docs.gitlab.com/ee/user/project/merge_requests/license_management.html) +**[ULTIMATE]** and [Dependency +Scanning](https://docs.gitlab.com/ee/user/project/merge_requests/dependency_scanning.html) +**[ULTIMATE]** should be activated on all projects to ensure new dependencies +security status and license compatibility. + +### Modules + +Since Go 1.11, a standard dependency system is available behind the name [Go +Modules](https://github.com/golang/go/wiki/Modules). It provides a way to +define and lock dependencies for reproducible builds. It should be used +whenever possible. + +There was a [bug on modules +checksums](https://github.com/golang/go/issues/29278) in Go < v1.11.4, so make +sure to use at least this version to avoid `checksum mismatch` errors. + +### ORM + +We don't use object-relational mapping libraries (ORMs) at GitLab (except +[ActiveRecord](https://guides.rubyonrails.org/active_record_basics.html) in +Ruby on Rails). Projects can be structured with services to avoid them. +[PQ](https://github.com/lib/pq) should be enough to interact with PostgreSQL +databases. + +### Migrations + +In the rare event of managing a hosted database, it's necessary to use a +migration system like ActiveRecord is providing. A simple library like +[Journey](https://github.com/db-journey/journey), designed to be used in +`postgres` containers, can be deployed as long-running pods. New versions will +deploy a new pod, migrating the data automatically. + +## Testing + +We should not use any specific library or framework for testing, as the +[standard library](https://golang.org/pkg/) provides already everything to get +started. For example, some external dependencies might be worth considering in +case we decide to use a specific library or framework: + +- [Testify](https://github.com/stretchr/testify) +- [httpexpect](https://github.com/gavv/httpexpect) + +Use [subtests](https://blog.golang.org/subtests) whenever possible to improve +code readability and test output. + +### Benchmarks + +Programs handling a lot of IO or complex operations should always include +[benchmarks](https://golang.org/pkg/testing/#hdr-Benchmarks), to ensure +performance consistency over time. + +## CLIs + +Every Go program is launched from the command line. +[cli](https://github.com/urfave/cli) is a convenient package to create command +line apps. It should be used whether the project is a daemon or a simple cli +tool. Flags can be mapped to [environment +variables](https://github.com/urfave/cli#values-from-the-environment) directly, +which documents and centralizes at the same time all the possible command line +interactions with the program. Don't use `os.GetEnv`, it hides variables deep +in the code. + +## Daemons + +### Logging + +The usage of a logging library is strongly recommended for daemons. Even though +there is a `log` package in the standard library, we generally use +[logrus](https://github.com/sirupsen/logrus). Its plugin ("hooks") system +makes it a powerful logging library, with the ability to add notifiers and +formatters at the logger level directly. + +### Tracing and Correlation + +[LabKit](https://gitlab.com/gitlab-org/labkit) is a place to keep common +libraries for Go services. Currently it's vendored into two projects: +Workhorse and Gitaly, and it exports two main (but related) pieces of +functionality: + +- [`gitlab.com/gitlab-org/labkit/correlation`](https://gitlab.com/gitlab-org/labkit/tree/master/correlation): + for propagating and extracting correlation ids between services. +- [`gitlab.com/gitlab-org/labkit/tracing`](https://gitlab.com/gitlab-org/labkit/tree/master/tracing): + for instrumenting Go libraries for distributed tracing. + +This gives us a thin abstraction over underlying implementations that is +consistent across Workhorse, Gitaly, and, in future, other Go servers. For +example, in the case of `gitlab.com/gitlab-org/labkit/tracing` we can switch +from using Opentracing directly to using Zipkin or Gokit's own tracing wrapper +without changes to the application code, while still keeping the same +consistent configuration mechanism (i.e. the `GITLAB_TRACING` environment +variable). + +### Context + +Since daemons are long-running applications, they should have mechanisms to +manage cancellations, and avoid unnecessary resources consumption (which could +lead to DDOS vulnerabilities). [Go +Context](https://github.com/golang/go/wiki/CodeReviewComments#contexts) should +be used in functions that can block and passed as the first parameter. + +## Dockerfiles + +Every project should have a `Dockerfile` at the root of their repository, to +build and run the project. Since Go program are static binaries, they should +not require any external dependency, and shells in the final image are useless. +We encourage [Multistage +builds](https://docs.docker.com/develop/develop-images/multistage-build/): + +- They let the user build the project with the right Go version and + dependencies. +- They generate a small, self-contained image, derived from `Scratch`. + +Generated docker images should have the program at their `Entrypoint` to create +portable commands. That way, anyone can run the image, and without parameters +it will display its help message (if `cli` has been used). + +--- + +[Return to Development documentation](../README.md). diff --git a/doc/install/installation.md b/doc/install/installation.md index 1f65e3415d1..45a76bd1d01 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -139,8 +139,8 @@ Then select 'Internet Site' and press enter to confirm the hostname. The Ruby interpreter is required to run GitLab. -**Note:** The current supported Ruby (MRI) version is 2.3.x. GitLab 9.0 dropped -support for Ruby 2.1.x. +**Note:** The current supported Ruby (MRI) version is 2.5.x. GitLab 11.6 + dropped support for Ruby 2.4.x. The use of Ruby version managers such as [RVM], [rbenv] or [chruby] with GitLab in production, frequently leads to hard to diagnose problems. For example, diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 184c7418e75..22ed1d8e7b4 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -93,7 +93,7 @@ module Backup progress.puts "Error: #{e}".color(:red) end else - restore_repo_success = gitlab_shell.create_repository(project.repository_storage, project.disk_path) + restore_repo_success = gitlab_shell.create_project_repository(project) end if restore_repo_success diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 7aa02009aa0..b2ef04d23d7 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -12,6 +12,9 @@ module Gitlab # Scopes used for OpenID Connect OPENID_SCOPES = [:openid].freeze + # OpenID Connect profile scopes + PROFILE_SCOPES = [:profile, :email].freeze + # Default scopes for OAuth applications that don't define their own DEFAULT_SCOPES = [:api].freeze @@ -284,7 +287,7 @@ module Gitlab # Other available scopes def optional_scopes - available_scopes + OPENID_SCOPES - DEFAULT_SCOPES + available_scopes + OPENID_SCOPES + PROFILE_SCOPES - DEFAULT_SCOPES end def registry_scopes diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index eaead41a720..75a3f17f549 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -65,9 +65,9 @@ module Gitlab def import_wiki return if project.wiki.repository_exists? - disk_path = project.wiki.disk_path - import_url = project.import_url.sub(/\.git\z/, ".git/wiki") - gitlab_shell.import_repository(project.repository_storage, disk_path, import_url) + wiki = WikiFormatter.new(project) + + gitlab_shell.import_wiki_repository(project, wiki) rescue StandardError => e errors << { type: :wiki, errors: e.message } end diff --git a/lib/gitlab/bitbucket_import/wiki_formatter.rb b/lib/gitlab/bitbucket_import/wiki_formatter.rb new file mode 100644 index 00000000000..b8ff43b777b --- /dev/null +++ b/lib/gitlab/bitbucket_import/wiki_formatter.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module BitbucketImport + class WikiFormatter + attr_reader :project + + def initialize(project) + @project = project + end + + def disk_path + project.wiki.disk_path + end + + def full_path + project.wiki.full_path + end + + def import_url + project.import_url.sub(/\.git\z/, ".git/wiki") + end + end + end +end diff --git a/lib/gitlab/git/object_pool.rb b/lib/gitlab/git/object_pool.rb index 1c6242b444a..e93ca3e11f8 100644 --- a/lib/gitlab/git/object_pool.rb +++ b/lib/gitlab/git/object_pool.rb @@ -10,12 +10,13 @@ module Gitlab delegate :exists?, :size, to: :repository delegate :unlink_repository, :delete, to: :object_pool_service - attr_reader :storage, :relative_path, :source_repository + attr_reader :storage, :relative_path, :source_repository, :gl_project_path - def initialize(storage, relative_path, source_repository) + def initialize(storage, relative_path, source_repository, gl_project_path) @storage = storage @relative_path = relative_path @source_repository = source_repository + @gl_project_path = gl_project_path end def create @@ -31,12 +32,12 @@ module Gitlab end def to_gitaly_repository - Gitlab::GitalyClient::Util.repository(storage, relative_path, GL_REPOSITORY) + Gitlab::GitalyClient::Util.repository(storage, relative_path, GL_REPOSITORY, gl_project_path) end # Allows for reusing other RPCs by 'tricking' Gitaly to think its a repository def repository - @repository ||= Gitlab::Git::Repository.new(storage, relative_path, GL_REPOSITORY) + @repository ||= Gitlab::Git::Repository.new(storage, relative_path, GL_REPOSITORY, gl_project_path) end private diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 786c90f9272..54bbd531398 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -67,7 +67,7 @@ module Gitlab # Relative path of repo attr_reader :relative_path - attr_reader :storage, :gl_repository, :relative_path + attr_reader :storage, :gl_repository, :relative_path, :gl_project_path # This remote name has to be stable for all types of repositories that # can join an object pool. If it's structure ever changes, a migration @@ -78,10 +78,11 @@ module Gitlab # This initializer method is only used on the client side (gitlab-ce). # Gitaly-ruby uses a different initializer. - def initialize(storage, relative_path, gl_repository) + def initialize(storage, relative_path, gl_repository, gl_project_path) @storage = storage @relative_path = relative_path @gl_repository = gl_repository + @gl_project_path = gl_project_path @name = @relative_path.split("/").last end @@ -872,7 +873,7 @@ module Gitlab end def gitaly_repository - Gitlab::GitalyClient::Util.repository(@storage, @relative_path, @gl_repository) + Gitlab::GitalyClient::Util.repository(@storage, @relative_path, @gl_repository, @gl_project_path) end def gitaly_ref_client diff --git a/lib/gitlab/gitaly_client/util.rb b/lib/gitlab/gitaly_client/util.rb index dce5d6a8ad0..899921f76e4 100644 --- a/lib/gitlab/gitaly_client/util.rb +++ b/lib/gitlab/gitaly_client/util.rb @@ -4,7 +4,7 @@ module Gitlab module GitalyClient module Util class << self - def repository(repository_storage, relative_path, gl_repository) + def repository(repository_storage, relative_path, gl_repository, gl_project_path) git_env = Gitlab::Git::HookEnv.all(gl_repository) git_object_directory = git_env['GIT_OBJECT_DIRECTORY_RELATIVE'].presence git_alternate_object_directories = Array.wrap(git_env['GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE']) @@ -14,14 +14,16 @@ module Gitlab relative_path: relative_path, gl_repository: gl_repository.to_s, git_object_directory: git_object_directory.to_s, - git_alternate_object_directories: git_alternate_object_directories + git_alternate_object_directories: git_alternate_object_directories, + gl_project_path: gl_project_path ) end def git_repository(gitaly_repository) Gitlab::Git::Repository.new(gitaly_repository.storage_name, gitaly_repository.relative_path, - gitaly_repository.gl_repository) + gitaly_repository.gl_repository, + gitaly_repository.gl_project_path) end end end diff --git a/lib/gitlab/github_import/importer/repository_importer.rb b/lib/gitlab/github_import/importer/repository_importer.rb index bc3ea9e9226..e2dfb00dcc5 100644 --- a/lib/gitlab/github_import/importer/repository_importer.rb +++ b/lib/gitlab/github_import/importer/repository_importer.rb @@ -6,11 +6,12 @@ module Gitlab class RepositoryImporter include Gitlab::ShellAdapter - attr_reader :project, :client + attr_reader :project, :client, :wiki_formatter def initialize(project, client) @project = project @client = client + @wiki_formatter = ::Gitlab::LegacyGithubImport::WikiFormatter.new(project) end # Returns true if we should import the wiki for the project. @@ -57,9 +58,7 @@ module Gitlab end def import_wiki_repository - wiki_path = "#{project.disk_path}.wiki" - - gitlab_shell.import_repository(project.repository_storage, wiki_path, wiki_url) + gitlab_shell.import_wiki_repository(project, wiki_formatter) true rescue Gitlab::Shell::Error => e @@ -72,7 +71,7 @@ module Gitlab end def wiki_url - project.import_url.sub(/\.git\z/, '.wiki.git') + wiki_formatter.import_url end def update_clone_time diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 4fbb87385c3..5ff415b6126 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -88,9 +88,10 @@ module Gitlab def create_cached_signature! using_keychain do |gpg_key| - signature = GpgSignature.new(attributes(gpg_key)) - signature.save! unless Gitlab::Database.read_only? - signature + attributes = attributes(gpg_key) + break GpgSignature.new(attributes) if Gitlab::Database.read_only? + + GpgSignature.safe_create!(attributes) end end diff --git a/lib/gitlab/legacy_github_import/importer.rb b/lib/gitlab/legacy_github_import/importer.rb index c526d31a591..f3323c98af2 100644 --- a/lib/gitlab/legacy_github_import/importer.rb +++ b/lib/gitlab/legacy_github_import/importer.rb @@ -267,7 +267,7 @@ module Gitlab def import_wiki unless project.wiki.repository_exists? wiki = WikiFormatter.new(project) - gitlab_shell.import_repository(project.repository_storage, wiki.disk_path, wiki.import_url) + gitlab_shell.import_wiki_repository(project, wiki) end rescue Gitlab::Shell::Error => e # GitHub error message when the wiki repo has not been created, diff --git a/lib/gitlab/legacy_github_import/wiki_formatter.rb b/lib/gitlab/legacy_github_import/wiki_formatter.rb index ea52be5ee0f..cf1e21ad1e1 100644 --- a/lib/gitlab/legacy_github_import/wiki_formatter.rb +++ b/lib/gitlab/legacy_github_import/wiki_formatter.rb @@ -13,6 +13,10 @@ module Gitlab project.wiki.disk_path end + def full_path + project.wiki.full_path + end + def import_url project.import_url.sub(/\.git\z/, ".wiki.git") end diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index bdf21cf3134..1153e69d3de 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -64,27 +64,48 @@ module Gitlab end end + # Convenience methods for initializing a new repository with a Project model. + def create_project_repository(project) + create_repository(project.repository_storage, project.disk_path, project.full_path) + end + + def create_wiki_repository(project) + create_repository(project.repository_storage, project.wiki.disk_path, project.wiki.full_path) + end + # Init new repository # # storage - the shard key - # name - project disk path + # disk_path - project disk path + # gl_project_path - project name # # Ex. - # create_repository("default", "gitlab/gitlab-ci") + # create_repository("default", "path/to/gitlab-ci", "gitlab/gitlab-ci") # - def create_repository(storage, name) - relative_path = name.dup + def create_repository(storage, disk_path, gl_project_path) + relative_path = disk_path.dup relative_path << '.git' unless relative_path.end_with?('.git') - repository = Gitlab::Git::Repository.new(storage, relative_path, '') + # During creation of a repository, gl_repository may not be known + # because that depends on a yet-to-be assigned project ID in the + # database (e.g. project-1234), so for now it is blank. + repository = Gitlab::Git::Repository.new(storage, relative_path, '', gl_project_path) wrapped_gitaly_errors { repository.gitaly_repository_client.create_repository } true rescue => err # Once the Rugged codes gets removes this can be improved - Rails.logger.error("Failed to add repository #{storage}/#{name}: #{err}") + Rails.logger.error("Failed to add repository #{storage}/#{disk_path}: #{err}") false end + def import_wiki_repository(project, wiki_formatter) + import_repository(project.repository_storage, wiki_formatter.disk_path, wiki_formatter.import_url, project.wiki.full_path) + end + + def import_project_repository(project) + import_repository(project.repository_storage, project.disk_path, project.import_url, project.full_path) + end + # Import repository # # storage - project's storage name @@ -94,13 +115,13 @@ module Gitlab # Ex. # import_repository("nfs-file06", "gitlab/gitlab-ci", "https://gitlab.com/gitlab-org/gitlab-test.git") # - def import_repository(storage, name, url) + def import_repository(storage, name, url, gl_project_path) if url.start_with?('.', '/') raise Error.new("don't use disk paths with import_repository: #{url.inspect}") end relative_path = "#{name}.git" - cmd = GitalyGitlabProjects.new(storage, relative_path) + cmd = GitalyGitlabProjects.new(storage, relative_path, gl_project_path) success = cmd.import_project(url, git_timeout) raise Error, cmd.output unless success @@ -125,18 +146,13 @@ module Gitlab end # Fork repository to new path - # forked_from_storage - forked-from project's storage name - # forked_from_disk_path - project disk relative path - # forked_to_storage - forked-to project's storage name - # forked_to_disk_path - forked project disk relative path - # - # Ex. - # fork_repository("nfs-file06", "gitlab/gitlab-ci", "nfs-file07", "new-namespace/gitlab-ci") - def fork_repository(forked_from_storage, forked_from_disk_path, forked_to_storage, forked_to_disk_path) - forked_from_relative_path = "#{forked_from_disk_path}.git" - fork_args = [forked_to_storage, "#{forked_to_disk_path}.git"] + # source_project - forked-from Project + # target_project - forked-to Project + def fork_repository(source_project, target_project) + forked_from_relative_path = "#{source_project.disk_path}.git" + fork_args = [target_project.repository_storage, "#{target_project.disk_path}.git", target_project.full_path] - GitalyGitlabProjects.new(forked_from_storage, forked_from_relative_path).fork_repository(*fork_args) + GitalyGitlabProjects.new(source_project.repository_storage, forked_from_relative_path, source_project.full_path).fork_repository(*fork_args) end # Removes a repository from file system, using rm_diretory which is an alias @@ -397,16 +413,17 @@ module Gitlab end class GitalyGitlabProjects - attr_reader :shard_name, :repository_relative_path, :output + attr_reader :shard_name, :repository_relative_path, :output, :gl_project_path - def initialize(shard_name, repository_relative_path) + def initialize(shard_name, repository_relative_path, gl_project_path) @shard_name = shard_name @repository_relative_path = repository_relative_path @output = '' + @gl_project_path = gl_project_path end def import_project(source, _timeout) - raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil) + raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil, gl_project_path) Gitlab::GitalyClient::RepositoryService.new(raw_repository).import_repository(source) true @@ -415,9 +432,9 @@ module Gitlab false end - def fork_repository(new_shard_name, new_repository_relative_path) - target_repository = Gitlab::Git::Repository.new(new_shard_name, new_repository_relative_path, nil) - raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil) + def fork_repository(new_shard_name, new_repository_relative_path, new_project_name) + target_repository = Gitlab::Git::Repository.new(new_shard_name, new_repository_relative_path, nil, new_project_name) + raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil, gl_project_path) Gitlab::GitalyClient::RepositoryService.new(target_repository).fork_repository(raw_repository) rescue GRPC::BadStatus => e diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 6464398cea1..844c61f1ace 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -107,8 +107,11 @@ describe Projects::ErrorTrackingController do let(:http_status) { :no_content } before do - expect(list_issues_service).to receive(:execute) - .and_return(status: :error, message: error_message, http_status: http_status) + expect(list_issues_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) end it 'returns http_status with message' do @@ -122,6 +125,113 @@ describe Projects::ErrorTrackingController do end end + describe 'POST #list_projects' do + context 'with insufficient permissions' do + before do + project.add_guest(user) + end + + it 'returns 404' do + post :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with an anonymous user' do + before do + sign_out(user) + end + + it 'redirects to sign-in page' do + post :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'with authorized user' do + let(:list_projects_service) { spy(:list_projects_service) } + let(:sentry_project) { build(:error_tracking_project) } + + let(:permitted_params) do + ActionController::Parameters.new( + list_projects_params[:error_tracking_setting] + ).permit! + end + + before do + allow(ErrorTracking::ListProjectsService) + .to receive(:new).with(project, user, permitted_params) + .and_return(list_projects_service) + end + + context 'service result is successful' do + before do + expect(list_projects_service).to receive(:execute) + .and_return(status: :success, projects: [sentry_project]) + end + + it 'returns a list of projects' do + post :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/list_projects') + expect(json_response['projects']).to eq([sentry_project].as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(list_projects_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(list_projects_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) + end + + it 'returns http_status with message' do + get :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + + private + + def list_projects_params(opts = {}) + project_params( + format: :json, + error_tracking_setting: { + api_host: 'gitlab.com', + token: 'token' + } + ) + end + end + private def project_params(opts = {}) diff --git a/spec/javascripts/notes/components/noteable_note_spec.js b/spec/javascripts/notes/components/noteable_note_spec.js index 4a143ef089a..8ade6fc2ced 100644 --- a/spec/javascripts/notes/components/noteable_note_spec.js +++ b/spec/javascripts/notes/components/noteable_note_spec.js @@ -1,105 +1,64 @@ +import $ from 'jquery'; import _ from 'underscore'; -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vue from 'vue'; import createStore from '~/notes/stores'; import issueNote from '~/notes/components/noteable_note.vue'; -import NoteHeader from '~/notes/components/note_header.vue'; -import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; -import NoteActions from '~/notes/components/note_actions.vue'; -import NoteBody from '~/notes/components/note_body.vue'; import { noteableDataMock, notesDataMock, note } from '../mock_data'; describe('issue_note', () => { let store; - let wrapper; + let vm; beforeEach(() => { + const Component = Vue.extend(issueNote); + store = createStore(); store.dispatch('setNoteableData', noteableDataMock); store.dispatch('setNotesData', notesDataMock); - const localVue = createLocalVue(); - wrapper = shallowMount(issueNote, { + vm = new Component({ store, propsData: { note, }, - sync: false, - localVue, - }); + }).$mount(); }); afterEach(() => { - wrapper.destroy(); + vm.$destroy(); }); it('should render user information', () => { - const { author } = note; - const avatar = wrapper.find(UserAvatarLink); - const avatarProps = avatar.props(); - - expect(avatarProps.linkHref).toBe(author.path); - expect(avatarProps.imgSrc).toBe(author.avatar_url); - expect(avatarProps.imgAlt).toBe(author.name); - expect(avatarProps.imgSize).toBe(40); + expect(vm.$el.querySelector('.user-avatar-link img').getAttribute('src')).toEqual( + note.author.avatar_url, + ); }); it('should render note header content', () => { - const noteHeader = wrapper.find(NoteHeader); - const noteHeaderProps = noteHeader.props(); + const el = vm.$el.querySelector('.note-header .note-header-author-name'); - expect(noteHeaderProps.author).toEqual(note.author); - expect(noteHeaderProps.createdAt).toEqual(note.created_at); - expect(noteHeaderProps.noteId).toEqual(note.id); + expect(el.textContent.trim()).toEqual(note.author.name); }); it('should render note actions', () => { - const { author } = note; - const noteActions = wrapper.find(NoteActions); - const noteActionsProps = noteActions.props(); - - expect(noteActionsProps.authorId).toBe(author.id); - expect(noteActionsProps.noteId).toBe(note.id); - expect(noteActionsProps.noteUrl).toBe(note.noteable_note_url); - expect(noteActionsProps.accessLevel).toBe(note.human_access); - expect(noteActionsProps.canEdit).toBe(note.current_user.can_edit); - expect(noteActionsProps.canAwardEmoji).toBe(note.current_user.can_award_emoji); - expect(noteActionsProps.canDelete).toBe(note.current_user.can_edit); - expect(noteActionsProps.canReportAsAbuse).toBe(true); - expect(noteActionsProps.canResolve).toBe(false); - expect(noteActionsProps.reportAbusePath).toBe(note.report_abuse_path); - expect(noteActionsProps.resolvable).toBe(false); - expect(noteActionsProps.isResolved).toBe(false); - expect(noteActionsProps.isResolving).toBe(false); - expect(noteActionsProps.resolvedBy).toEqual({}); + expect(vm.$el.querySelector('.note-actions')).toBeDefined(); }); it('should render issue body', () => { - const noteBody = wrapper.find(NoteBody); - const noteBodyProps = noteBody.props(); - - expect(noteBodyProps.note).toEqual(note); - expect(noteBodyProps.line).toBe(null); - expect(noteBodyProps.canEdit).toBe(note.current_user.can_edit); - expect(noteBodyProps.isEditing).toBe(false); - expect(noteBodyProps.helpPagePath).toBe(''); + expect(vm.$el.querySelector('.note-text').innerHTML).toEqual(note.note_html); }); it('prevents note preview xss', done => { const imgSrc = 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'; const noteBody = `<img src="${imgSrc}" onload="alert(1)" />`; const alertSpy = spyOn(window, 'alert'); - store.hotUpdate({ - actions: { - updateNote() {}, - }, - }); - const noteBodyComponent = wrapper.find(NoteBody); + vm.updateNote = () => new Promise($.noop); - noteBodyComponent.vm.$emit('handleFormUpdate', noteBody, null, () => {}); + vm.formUpdateHandler(noteBody, null, $.noop); setTimeout(() => { expect(alertSpy).not.toHaveBeenCalled(); - expect(wrapper.vm.note.note_html).toEqual(_.escape(noteBody)); + expect(vm.note.note_html).toEqual(_.escape(noteBody)); done(); }, 0); }); @@ -107,23 +66,17 @@ describe('issue_note', () => { describe('cancel edit', () => { it('restores content of updated note', done => { const noteBody = 'updated note text'; - store.hotUpdate({ - actions: { - updateNote() {}, - }, - }); - const noteBodyComponent = wrapper.find(NoteBody); - noteBodyComponent.vm.resetAutoSave = () => {}; + vm.updateNote = () => Promise.resolve(); - noteBodyComponent.vm.$emit('handleFormUpdate', noteBody, null, () => {}); + vm.formUpdateHandler(noteBody, null, $.noop); setTimeout(() => { - expect(wrapper.vm.note.note_html).toEqual(noteBody); + expect(vm.note.note_html).toEqual(noteBody); - noteBodyComponent.vm.$emit('cancelForm'); + vm.formCancelHandler(); setTimeout(() => { - expect(wrapper.vm.note.note_html).toEqual(noteBody); + expect(vm.note.note_html).toEqual(noteBody); done(); }); diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 236808c0b69..a4a6338961e 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -19,7 +19,7 @@ describe Gitlab::Auth do it 'optional_scopes contains all non-default scopes' do stub_container_registry_config(enabled: true) - expect(subject.optional_scopes).to eq %i[read_user sudo read_repository read_registry openid] + expect(subject.optional_scopes).to eq %i[read_user sudo read_repository read_registry openid profile email] end context 'registry_scopes' do diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index afd8f5da39f..a07c5371134 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -61,7 +61,7 @@ describe ::Gitlab::BareRepositoryImport::Repository do let(:wiki_path) { File.join(root_path, "#{hashed_path}.wiki.git") } before do - gitlab_shell.create_repository(repository_storage, hashed_path) + gitlab_shell.create_repository(repository_storage, hashed_path, 'group/project') Gitlab::GitalyClient::StorageSettings.allow_disk_access do repository = Rugged::Repository.new(repo_path) repository.config['gitlab.fullpath'] = 'to/repo' diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index 0def685f177..c432cc223b9 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -218,7 +218,7 @@ describe Gitlab::BitbucketImport::Importer do describe 'wiki import' do it 'is skipped when the wiki exists' do expect(project.wiki).to receive(:repository_exists?) { true } - expect(importer.gitlab_shell).not_to receive(:import_repository) + expect(importer.gitlab_shell).not_to receive(:import_wiki_repository) importer.execute @@ -227,11 +227,7 @@ describe Gitlab::BitbucketImport::Importer do it 'imports to the project disk_path' do expect(project.wiki).to receive(:repository_exists?) { false } - expect(importer.gitlab_shell).to receive(:import_repository).with( - project.repository_storage, - project.wiki.disk_path, - project.import_url + '/wiki' - ) + expect(importer.gitlab_shell).to receive(:import_wiki_repository) importer.execute diff --git a/spec/lib/gitlab/bitbucket_import/wiki_formatter_spec.rb b/spec/lib/gitlab/bitbucket_import/wiki_formatter_spec.rb new file mode 100644 index 00000000000..795fd069ab2 --- /dev/null +++ b/spec/lib/gitlab/bitbucket_import/wiki_formatter_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::BitbucketImport::WikiFormatter do + let(:project) do + create(:project, + namespace: create(:namespace, path: 'gitlabhq'), + import_url: 'https://xxx@bitbucket.org/gitlabhq/sample.gitlabhq.git') + end + + subject(:wiki) { described_class.new(project) } + + describe '#disk_path' do + it 'appends .wiki to disk path' do + expect(wiki.disk_path).to eq project.wiki.disk_path + end + end + + describe '#full_path' do + it 'appends .wiki to project path' do + expect(wiki.full_path).to eq project.wiki.full_path + end + end + + describe '#import_url' do + it 'returns URL of the wiki repository' do + expect(wiki.import_url).to eq 'https://xxx@bitbucket.org/gitlabhq/sample.gitlabhq.git/wiki' + end + end +end diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index e704d1c673c..0010c0304eb 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe Gitlab::Git::Blame, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:blame) do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index 1bcec04d28f..a1b5cea88c0 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Blob, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:rugged) do Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) end diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 0df282d0ae3..0764e525ede 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Branch, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:rugged) do Rugged::Repository.new(File.join(TestEnv.repos_path, repository.relative_path)) end @@ -64,7 +64,7 @@ describe Gitlab::Git::Branch, :seed_helper do context 'with active, stale and future branches' do let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') + Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') end let(:user) { create(:user) } diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index db68062e433..2611ebed25b 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Commit, :seed_helper do include GitHelpers - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:rugged_repo) do Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) end @@ -146,7 +146,7 @@ describe Gitlab::Git::Commit, :seed_helper do end context 'with broken repo' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '', 'group/project') } it 'returns nil' do expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil diff --git a/spec/lib/gitlab/git/compare_spec.rb b/spec/lib/gitlab/git/compare_spec.rb index 771c71a16a9..65dfb93d0db 100644 --- a/spec/lib/gitlab/git/compare_spec.rb +++ b/spec/lib/gitlab/git/compare_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Compare, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:compare) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: false) } let(:compare_straight) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: true) } diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 8a4415506c4..1d22329b670 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Diff, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:gitaly_diff) do Gitlab::GitalyClient::Diff.new( from_path: '.gitmodules', diff --git a/spec/lib/gitlab/git/remote_repository_spec.rb b/spec/lib/gitlab/git/remote_repository_spec.rb index 53ed7c5a13a..e166628d4ca 100644 --- a/spec/lib/gitlab/git/remote_repository_spec.rb +++ b/spec/lib/gitlab/git/remote_repository_spec.rb @@ -1,15 +1,15 @@ require 'spec_helper' describe Gitlab::Git::RemoteRepository, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } subject { described_class.new(repository) } describe '#empty?' do using RSpec::Parameterized::TableSyntax where(:repository, :result) do - Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') | false - Gitlab::Git::Repository.new('default', 'does-not-exist.git', '') | true + Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') | false + Gitlab::Git::Repository.new('default', 'does-not-exist.git', '', 'group/project') | true end with_them do @@ -44,11 +44,11 @@ describe Gitlab::Git::RemoteRepository, :seed_helper do using RSpec::Parameterized::TableSyntax where(:other_repository, :result) do - repository | true - Gitlab::Git::Repository.new(repository.storage, repository.relative_path, '') | true - Gitlab::Git::Repository.new('broken', TEST_REPO_PATH, '') | false - Gitlab::Git::Repository.new(repository.storage, 'wrong/relative-path.git', '') | false - Gitlab::Git::Repository.new('broken', 'wrong/relative-path.git', '') | false + repository | true + Gitlab::Git::Repository.new(repository.storage, repository.relative_path, '', 'group/project') | true + Gitlab::Git::Repository.new('broken', TEST_REPO_PATH, '', 'group/project') | false + Gitlab::Git::Repository.new(repository.storage, 'wrong/relative-path.git', '', 'group/project') | false + Gitlab::Git::Repository.new('broken', 'wrong/relative-path.git', '', 'group/project') | false end with_them do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index a19e3e84f83..caf279edc97 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -19,8 +19,8 @@ describe Gitlab::Git::Repository, :seed_helper do end end - let(:mutable_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:mutable_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:repository_path) { File.join(TestEnv.repos_path, repository.relative_path) } let(:repository_rugged) { Rugged::Repository.new(repository_path) } let(:storage_path) { TestEnv.repos_path } @@ -434,13 +434,13 @@ describe Gitlab::Git::Repository, :seed_helper do describe '#fetch_repository_as_mirror' do let(:new_repository) do - Gitlab::Git::Repository.new('default', 'my_project.git', '') + Gitlab::Git::Repository.new('default', 'my_project.git', '', 'group/project') end subject { new_repository.fetch_repository_as_mirror(repository) } before do - Gitlab::Shell.new.create_repository('default', 'my_project') + Gitlab::Shell.new.create_repository('default', 'my_project', 'group/project') end after do @@ -1230,7 +1230,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#gitattribute' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') } after do ensure_seeds @@ -1249,7 +1249,7 @@ describe Gitlab::Git::Repository, :seed_helper do end context 'without gitattributes file' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } it 'returns nil' do expect(repository.gitattribute("README.md", 'gitlab-language')).to eq(nil) @@ -1513,7 +1513,7 @@ describe Gitlab::Git::Repository, :seed_helper do context 'repository does not exist' do it 'raises NoRepository and does not call Gitaly WriteConfig' do - repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '') + repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '', 'group/project') expect(repository.gitaly_repository_client).not_to receive(:write_config) @@ -1803,7 +1803,7 @@ describe Gitlab::Git::Repository, :seed_helper do out: '/dev/null', err: '/dev/null') - empty_repo = described_class.new('default', 'empty-repo.git', '') + empty_repo = described_class.new('default', 'empty-repo.git', '', 'group/empty-repo') expect(empty_repo.checksum).to eq '0000000000000000000000000000000000000000' end @@ -1818,13 +1818,13 @@ describe Gitlab::Git::Repository, :seed_helper do File.truncate(File.join(storage_path, 'non-valid.git/HEAD'), 0) - non_valid = described_class.new('default', 'non-valid.git', '') + non_valid = described_class.new('default', 'non-valid.git', '', 'a/non-valid') expect { non_valid.checksum }.to raise_error(Gitlab::Git::Repository::InvalidRepository) end it 'raises Gitlab::Git::Repository::NoRepository error when there is no repo' do - broken_repo = described_class.new('default', 'a/path.git', '') + broken_repo = described_class.new('default', 'a/path.git', '', 'a/path') expect { broken_repo.checksum }.to raise_error(Gitlab::Git::Repository::NoRepository) end diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index b51e3879f49..4c0291f64f0 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tag, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } describe '#tags' do describe 'first tag' do diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index bec875fb03d..4a4d69490a3 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tree, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } context :repo do let(:tree) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID) } diff --git a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb index aff47599ad6..d5508dbff5d 100644 --- a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::GitalyClient::RemoteService do end describe '#fetch_internal_remote' do - let(:remote_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:remote_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') } it 'sends an fetch_internal_remote message and returns the result value' do expect_any_instance_of(Gitaly::RemoteService::Stub) diff --git a/spec/lib/gitlab/gitaly_client/util_spec.rb b/spec/lib/gitlab/gitaly_client/util_spec.rb index 550db6db6d9..78a5e195ad1 100644 --- a/spec/lib/gitlab/gitaly_client/util_spec.rb +++ b/spec/lib/gitlab/gitaly_client/util_spec.rb @@ -7,6 +7,7 @@ describe Gitlab::GitalyClient::Util do let(:gl_repository) { 'project-1' } let(:git_object_directory) { '.git/objects' } let(:git_alternate_object_directory) { ['/dir/one', '/dir/two'] } + let(:gl_project_path) { 'namespace/myproject' } let(:git_env) do { 'GIT_OBJECT_DIRECTORY_RELATIVE' => git_object_directory, @@ -15,7 +16,7 @@ describe Gitlab::GitalyClient::Util do end subject do - described_class.repository(repository_storage, relative_path, gl_repository) + described_class.repository(repository_storage, relative_path, gl_repository, gl_project_path) end it 'creates a Gitaly::Repository with the given data' do @@ -27,6 +28,7 @@ describe Gitlab::GitalyClient::Util do expect(subject.gl_repository).to eq(gl_repository) expect(subject.git_object_directory).to eq(git_object_directory) expect(subject.git_alternate_object_directories).to eq(git_alternate_object_directory) + expect(subject.gl_project_path).to eq(gl_project_path) end end end diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb index 77f5b2ffa37..47233ea6ee2 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -5,6 +5,14 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do let(:import_state) { double(:import_state) } let(:client) { double(:client) } + let(:wiki) do + double( + :wiki, + disk_path: 'foo.wiki', + full_path: 'group/foo.wiki' + ) + end + let(:project) do double( :project, @@ -15,7 +23,9 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do repository: repository, create_wiki: true, import_state: import_state, - lfs_enabled?: true + full_path: 'group/foo', + lfs_enabled?: true, + wiki: wiki ) end @@ -195,7 +205,7 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do it 'imports the wiki repository' do expect(importer.gitlab_shell) .to receive(:import_repository) - .with('foo', 'foo.wiki', 'foo.wiki.git') + .with('foo', 'foo.wiki', 'foo.wiki.git', 'group/foo.wiki') expect(importer.import_wiki_repository).to eq(true) end diff --git a/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb index 7723533aee2..7519707293c 100644 --- a/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb @@ -10,11 +10,17 @@ describe Gitlab::LegacyGithubImport::WikiFormatter do subject(:wiki) { described_class.new(project) } describe '#disk_path' do - it 'appends .wiki to project path' do + it 'appends .wiki to disk path' do expect(wiki.disk_path).to eq project.wiki.disk_path end end + describe '#full_path' do + it 'appends .wiki to project path' do + expect(wiki.full_path).to eq project.wiki.full_path + end + end + describe '#import_url' do it 'returns URL of the wiki repository' do expect(wiki.import_url).to eq 'https://xxx@github.com/gitlabhq/sample.gitlabhq.wiki.git' diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 6ce9d515a0f..033e1bf81a1 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -412,7 +412,7 @@ describe Gitlab::Shell do end it 'creates a repository' do - expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_truthy + expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_truthy expect(File.stat(created_path).mode & 0o777).to eq(0o770) @@ -427,7 +427,7 @@ describe Gitlab::Shell do # should cause #create_repository to fail. FileUtils.touch(created_path) - expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_falsy + expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_falsy end end @@ -474,13 +474,10 @@ describe Gitlab::Shell do end describe '#fork_repository' do + let(:target_project) { create(:project) } + subject do - gitlab_shell.fork_repository( - project.repository_storage, - project.disk_path, - 'nfs-file05', - 'fork/path' - ) + gitlab_shell.fork_repository(project, target_project) end it 'returns true when the command succeeds' do @@ -505,7 +502,7 @@ describe Gitlab::Shell do it 'returns true when the command succeeds' do expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository).with(import_url) - result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url, project.full_path) expect(result).to be_truthy end @@ -516,7 +513,7 @@ describe Gitlab::Shell do expect_any_instance_of(Gitlab::Shell::GitalyGitlabProjects).to receive(:output) { 'error'} expect do - gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url, project.full_path) end.to raise_error(Gitlab::Shell::Error, "error") end end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index ca23f581fdc..fd25132ed3a 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -11,7 +11,7 @@ describe ApplicationRecord do end end - describe '#safe_find_or_create_by' do + describe '.safe_find_or_create_by' do it 'creates the user avoiding race conditions' do expect(Suggestion).to receive(:find_or_create_by).and_raise(ActiveRecord::RecordNotUnique) allow(Suggestion).to receive(:find_or_create_by).and_call_original @@ -20,4 +20,17 @@ describe ApplicationRecord do .to change { Suggestion.count }.by(1) end end + + describe '.safe_find_or_create_by!' do + it 'creates a record using safe_find_or_create_by' do + expect(Suggestion).to receive(:find_or_create_by).and_call_original + + expect(Suggestion.safe_find_or_create_by!(build(:suggestion).attributes)) + .to be_a(Suggestion) + end + + it 'raises a validation error if the record was not persisted' do + expect { Suggestion.find_or_create_by!(note: nil) }.to raise_error(ActiveRecord::RecordInvalid) + end + end end diff --git a/spec/models/gpg_signature_spec.rb b/spec/models/gpg_signature_spec.rb index cdd7dea2064..e90319c39b1 100644 --- a/spec/models/gpg_signature_spec.rb +++ b/spec/models/gpg_signature_spec.rb @@ -23,6 +23,41 @@ RSpec.describe GpgSignature do it { is_expected.to validate_presence_of(:gpg_key_primary_keyid) } end + describe '.safe_create!' do + let(:attributes) do + { + commit_sha: commit_sha, + project: project, + gpg_key_primary_keyid: gpg_key.keyid + } + end + + it 'finds a signature by commit sha if it existed' do + gpg_signature + + expect(described_class.safe_create!(commit_sha: commit_sha)).to eq(gpg_signature) + end + + it 'creates a new signature if it was not found' do + expect { described_class.safe_create!(attributes) }.to change { described_class.count }.by(1) + end + + it 'assigns the correct attributes when creating' do + signature = described_class.safe_create!(attributes) + + expect(signature.project).to eq(project) + expect(signature.commit_sha).to eq(commit_sha) + expect(signature.gpg_key_primary_keyid).to eq(gpg_key.keyid) + end + + it 'does not raise an error in case of a race condition' do + expect(described_class).to receive(:find_or_create_by).and_raise(ActiveRecord::RecordNotUnique) + allow(described_class).to receive(:find_or_create_by).and_call_original + + described_class.safe_create!(attributes) + end + end + describe '#commit' do it 'fetches the commit through the project' do expect_any_instance_of(Project).to receive(:commit).with(commit_sha).and_return(commit) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ae137aa7b78..c1767ed0535 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1765,7 +1765,7 @@ describe Project do context 'using a regular repository' do it 'creates the repository' do expect(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path) + .with(project.repository_storage, project.disk_path, project.full_path) .and_return(true) expect(project.repository).to receive(:after_create) @@ -1775,7 +1775,7 @@ describe Project do it 'adds an error if the repository could not be created' do expect(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path) + .with(project.repository_storage, project.disk_path, project.full_path) .and_return(false) expect(project.repository).not_to receive(:after_create) @@ -1808,7 +1808,7 @@ describe Project do .and_return(false) allow(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path) + .with(project.repository_storage, project.disk_path, project.full_path) .and_return(true) expect(project).to receive(:create_repository).with(force: true) @@ -1832,7 +1832,7 @@ describe Project do .and_return(false) expect(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path) + .with(project.repository_storage, project.disk_path, project.full_path) .and_return(true) project.ensure_repository diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 48a43801b9f..3ccc706edf2 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -7,7 +7,7 @@ describe ProjectWiki do let(:repository) { project.repository } let(:gitlab_shell) { Gitlab::Shell.new } let(:project_wiki) { described_class.new(project, user) } - let(:raw_repository) { Gitlab::Git::Repository.new(project.repository_storage, subject.disk_path + '.git', 'foo') } + let(:raw_repository) { Gitlab::Git::Repository.new(project.repository_storage, subject.disk_path + '.git', 'foo', 'group/project.wiki') } let(:commit) { project_wiki.repository.head_commit } subject { project_wiki } @@ -75,7 +75,7 @@ describe ProjectWiki do # Create a fresh project which will not have a wiki project_wiki = described_class.new(create(:project), user) gitlab_shell = double(:gitlab_shell) - allow(gitlab_shell).to receive(:create_repository) + allow(gitlab_shell).to receive(:create_wiki_repository) allow(project_wiki).to receive(:gitlab_shell).and_return(gitlab_shell) expect { project_wiki.send(:wiki) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 4978c43c9b5..f78760bf567 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2291,6 +2291,7 @@ describe Repository do expect(subject).to be_a(Gitlab::Git::Repository) expect(subject.relative_path).to eq(project.disk_path + '.git') expect(subject.gl_repository).to eq("project-#{project.id}") + expect(subject.gl_project_path).to eq(project.full_path) end context 'with a wiki repository' do @@ -2300,6 +2301,7 @@ describe Repository do expect(subject).to be_a(Gitlab::Git::Repository) expect(subject.relative_path).to eq(project.disk_path + '.wiki.git') expect(subject.gl_repository).to eq("wiki-#{project.id}") + expect(subject.gl_project_path).to eq(project.full_path) end end end diff --git a/spec/presenters/blob_presenter_spec.rb b/spec/presenters/blob_presenter_spec.rb index e85e7a41017..bb1db9a3d51 100644 --- a/spec/presenters/blob_presenter_spec.rb +++ b/spec/presenters/blob_presenter_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe BlobPresenter, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:git_blob) do Gitlab::Git::Blob.find( diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 2b148c1b563..2a455523e2c 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -35,7 +35,7 @@ describe 'OpenID Connect requests' do 'name' => 'Alice', 'nickname' => 'alice', 'email' => 'public@example.com', - 'email_verified' => true, + 'email_verified' => false, 'website' => 'https://example.com', 'profile' => 'http://localhost/alice', 'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png", @@ -111,6 +111,18 @@ describe 'OpenID Connect requests' do it 'does not include any unknown claims' do expect(json_response.keys).to eq %w[sub sub_legacy] + user_info_claims.keys end + + it 'includes email and email_verified claims' do + expect(json_response.keys).to include('email', 'email_verified') + end + + it 'has public email in email claim' do + expect(json_response['email']).to eq(user.public_email) + end + + it 'has false in email_verified claim' do + expect(json_response['email_verified']).to eq(false) + end end context 'ID token payload' do @@ -175,7 +187,35 @@ describe 'OpenID Connect requests' do expect(response).to have_gitlab_http_status(200) expect(json_response['issuer']).to eq('http://localhost') expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys') - expect(json_response['scopes_supported']).to eq(%w[api read_user sudo read_repository openid]) + expect(json_response['scopes_supported']).to eq(%w[api read_user sudo read_repository openid profile email]) + end + end + + context 'Application with OpenID and email scopes' do + let(:application) { create :oauth_application, scopes: 'openid email' } + + it 'token response includes an ID token' do + request_access_token! + + expect(json_response).to include 'id_token' + end + + context 'UserInfo payload' do + before do + request_user_info! + end + + it 'includes the email and email_verified claims' do + expect(json_response.keys).to include('email', 'email_verified') + end + + it 'has private email in email claim' do + expect(json_response['email']).to eq(user.email) + end + + it 'has true in email_verified claim' do + expect(json_response['email_verified']).to eq(true) + end end end end diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb new file mode 100644 index 00000000000..ee9c59e3f65 --- /dev/null +++ b/spec/services/error_tracking/list_projects_service_spec.rb @@ -0,0 +1,149 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ErrorTracking::ListProjectsService do + set(:user) { create(:user) } + set(:project) { create(:project) } + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + let(:new_api_host) { 'https://gitlab.com/' } + let(:new_token) { 'new-token' } + let(:params) { ActionController::Parameters.new(api_host: new_api_host, token: new_token) } + + let(:error_tracking_setting) do + create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) + end + + subject { described_class.new(project, user, params) } + + before do + project.add_reporter(user) + end + + describe '#execute' do + let(:result) { subject.execute } + + context 'with authorized user' do + before do + expect(project).to receive(:error_tracking_setting).at_least(:once) + .and_return(error_tracking_setting) + end + + context 'set model attributes to new values' do + let(:new_api_url) { new_api_host + 'api/0/projects/' } + + before do + expect(error_tracking_setting).to receive(:list_sentry_projects) + .and_return({ projects: [] }) + end + + it 'uses new api_url and token' do + subject.execute + + expect(error_tracking_setting.api_url).to eq(new_api_url) + expect(error_tracking_setting.token).to eq(new_token) + error_tracking_setting.reload + expect(error_tracking_setting.api_url).to eq(sentry_url) + expect(error_tracking_setting.token).to eq(token) + end + end + + context 'sentry client raises exception' do + before do + expect(error_tracking_setting).to receive(:list_sentry_projects) + .and_raise(Sentry::Client::Error, 'Sentry response error: 500') + end + + it 'returns error response' do + expect(result[:message]).to eq('Sentry response error: 500') + expect(result[:http_status]).to eq(:bad_request) + end + end + + context 'with invalid url' do + let(:params) do + ActionController::Parameters.new( + api_host: 'https://localhost', + token: new_token + ) + end + + before do + error_tracking_setting.enabled = false + end + + it 'returns error' do + expect(result[:message]).to start_with('Api url is blocked') + expect(error_tracking_setting).not_to be_valid + end + end + + context 'when list_sentry_projects returns projects' do + let(:projects) { [:list, :of, :projects] } + + before do + expect(error_tracking_setting) + .to receive(:list_sentry_projects).and_return(projects: projects) + end + + it 'returns the projects' do + expect(result).to eq(status: :success, projects: projects) + end + end + end + + context 'with unauthorized user' do + before do + project.add_guest(user) + end + + it 'returns error' do + expect(result).to include(status: :error, message: 'access denied') + end + end + + context 'with error tracking disabled' do + before do + expect(project).to receive(:error_tracking_setting).at_least(:once) + .and_return(error_tracking_setting) + expect(error_tracking_setting) + .to receive(:list_sentry_projects).and_return(projects: []) + + error_tracking_setting.enabled = false + end + + it 'ignores enabled flag' do + expect(result).to include(status: :success, projects: []) + end + end + + context 'error_tracking_setting is nil' do + let(:error_tracking_setting) { build(:project_error_tracking_setting) } + let(:new_api_url) { new_api_host + 'api/0/projects/' } + + before do + expect(project).to receive(:build_error_tracking_setting).once + .and_return(error_tracking_setting) + + expect(error_tracking_setting).to receive(:list_sentry_projects) + .and_return(projects: [:project1, :project2]) + end + + it 'builds a new error_tracking_setting' do + expect(project.error_tracking_setting).to be_nil + + expect(result[:projects]).to eq([:project1, :project2]) + + expect(error_tracking_setting.api_url).to eq(new_api_url) + expect(error_tracking_setting.token).to eq(new_token) + expect(error_tracking_setting.enabled).to be true + expect(error_tracking_setting.persisted?).to be false + expect(error_tracking_setting.project_id).not_to be_nil + + expect(project.error_tracking_setting).to be_nil + end + end + end +end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 54ce33dd103..d1b110b9806 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -116,7 +116,7 @@ describe Projects::CreateService, '#execute' do def wiki_repo(project) relative_path = ProjectWiki.new(project).disk_path + '.git' - Gitlab::Git::Repository.new(project.repository_storage, relative_path, 'foobar') + Gitlab::Git::Repository.new(project.repository_storage, relative_path, 'foobar', project.full_path) end end @@ -198,7 +198,7 @@ describe Projects::CreateService, '#execute' do context 'with legacy storage' do before do - gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing") + gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing", 'group/project') end after do @@ -234,7 +234,7 @@ describe Projects::CreateService, '#execute' do end before do - gitlab_shell.create_repository(repository_storage, hashed_path) + gitlab_shell.create_repository(repository_storage, hashed_path, 'group/project') end after do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 26e8d829345..23ec29cce7b 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -119,7 +119,7 @@ describe Projects::ForkService do let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path } before do - gitlab_shell.create_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}") + gitlab_shell.create_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}", "#{@to_user.namespace.full_path}/#{@from_project.path}") end after do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 766276fdba3..aae50d5307f 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -201,7 +201,7 @@ describe Projects::TransferService do before do group.add_owner(user) - unless gitlab_shell.create_repository(repository_storage, "#{group.full_path}/#{project.path}") + unless gitlab_shell.create_repository(repository_storage, "#{group.full_path}/#{project.path}", project.full_path) raise 'failed to add repository' end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 8adfc63222e..90eaea9c872 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -232,7 +232,7 @@ describe Projects::UpdateService do let(:project) { create(:project, :legacy_storage, :repository, creator: user, namespace: user.namespace) } before do - gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing") + gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing", user.namespace.full_path) end after do diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 781f91ac9ca..31bfe88d0bd 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -24,12 +24,7 @@ describe RepositoryForkWorker do end def expect_fork_repository - expect(shell).to receive(:fork_repository).with( - 'default', - project.disk_path, - forked_project.repository_storage, - forked_project.disk_path - ) + expect(shell).to receive(:fork_repository).with(project, forked_project) end describe 'when a worker was reset without cleanup' do |