diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4e8453726a..1f0e798e8a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -315,7 +315,7 @@ cloud-native-image: variables: GIT_DEPTH: "1" cache: {} - when: always + when: manual script: - gem install gitlab --no-document - CNG_PROJECT_PATH="gitlab-org/build/CNG" BUILD_TRIGGER_TOKEN=$CI_JOB_TOKEN ./scripts/trigger-build cng diff --git a/CHANGELOG.md b/CHANGELOG.md index 00b5b24034..a27def2724 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,43 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 11.8.9 (2019-04-25) + +### Security (5 changes) + +- Improve credentials sanitization on repository mirror integration. !3078 +- Stop sending emails to users who can't read commit. +- Escape path in new merge request mail. +- Only allow modification of content when note is edited. +- Upgrade Rails to 5.0.7.2. + + +## 11.8.8 (2019-04-23) + +### Fixed (5 changes) + +- Bring back Rugged implementation of find_commit. !25477 +- Fix bug in BitBucket imports with SHA shorter than 40 chars. !26050 +- Fix health checks not working behind load balancers. !26055 +- Fix error creating a merge request when diff includes a null byte. !26190 +- Avoid excessive recursive calls with Rugged TreeEntries. !26813 + +### Performance (1 change) + +- Bring back Rugged implementation of ListCommitsByOid. !27441 + +### Other (4 changes) + +- Bring back Rugged implementation of GetTreeEntries. !25674 +- Bring back Rugged implementation of CommitIsAncestor. !25702 +- Bring back Rugged implementation of TreeEntry. !25706 +- Bring back Rugged implementation of commit_tree_entry. !25896 + + +## 11.8.7 (2019-04-09) + +- No changes. + ## 11.8.6 (2019-03-28) ### Security (7 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 3989355915..0044d6cb96 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.20.0 +1.20.1 diff --git a/Gemfile b/Gemfile index a3b01c275c..80565aaff4 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ source 'https://rubygems.org' -gem 'rails', '5.0.7.1' +gem 'rails', '5.0.7.2' gem 'rails-deprecated_sanitizer', '~> 1.0.3' # Improves copy-on-write performance for MRI diff --git a/Gemfile.lock b/Gemfile.lock index 0b2bd2c96b..0ba8c3ef4c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,41 +4,41 @@ GEM RedCloth (4.3.2) abstract_type (0.0.7) ace-rails-ap (4.1.2) - actioncable (5.0.7.1) - actionpack (= 5.0.7.1) + actioncable (5.0.7.2) + actionpack (= 5.0.7.2) nio4r (>= 1.2, < 3.0) websocket-driver (~> 0.6.1) - actionmailer (5.0.7.1) - actionpack (= 5.0.7.1) - actionview (= 5.0.7.1) - activejob (= 5.0.7.1) + actionmailer (5.0.7.2) + actionpack (= 5.0.7.2) + actionview (= 5.0.7.2) + activejob (= 5.0.7.2) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.0.7.1) - actionview (= 5.0.7.1) - activesupport (= 5.0.7.1) + actionpack (5.0.7.2) + actionview (= 5.0.7.2) + activesupport (= 5.0.7.2) rack (~> 2.0) rack-test (~> 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.0.7.1) - activesupport (= 5.0.7.1) + actionview (5.0.7.2) + activesupport (= 5.0.7.2) builder (~> 3.1) erubis (~> 2.7.0) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.3) - activejob (5.0.7.1) - activesupport (= 5.0.7.1) + activejob (5.0.7.2) + activesupport (= 5.0.7.2) globalid (>= 0.3.6) - activemodel (5.0.7.1) - activesupport (= 5.0.7.1) - activerecord (5.0.7.1) - activemodel (= 5.0.7.1) - activesupport (= 5.0.7.1) + activemodel (5.0.7.2) + activesupport (= 5.0.7.2) + activerecord (5.0.7.2) + activemodel (= 5.0.7.2) + activesupport (= 5.0.7.2) arel (~> 7.0) activerecord_sane_schema_dumper (1.0) rails (>= 5, < 6) - activesupport (5.0.7.1) + activesupport (5.0.7.2) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -295,7 +295,7 @@ GEM omniauth (~> 1.3) pyu-ruby-sasl (>= 0.0.3.3, < 0.1) rubyntlm (~> 0.5) - globalid (0.4.1) + globalid (0.4.2) activesupport (>= 4.2.0) gon (6.2.0) actionpack (>= 3.0) @@ -385,7 +385,7 @@ GEM json (~> 1.8) multi_xml (>= 0.5.2) httpclient (2.8.3) - i18n (1.2.0) + i18n (1.6.0) concurrent-ruby (~> 1.0) icalendar (2.4.1) ice_nine (0.11.2) @@ -636,17 +636,17 @@ GEM rack rack-test (0.6.3) rack (>= 1.0) - rails (5.0.7.1) - actioncable (= 5.0.7.1) - actionmailer (= 5.0.7.1) - actionpack (= 5.0.7.1) - actionview (= 5.0.7.1) - activejob (= 5.0.7.1) - activemodel (= 5.0.7.1) - activerecord (= 5.0.7.1) - activesupport (= 5.0.7.1) + rails (5.0.7.2) + actioncable (= 5.0.7.2) + actionmailer (= 5.0.7.2) + actionpack (= 5.0.7.2) + actionview (= 5.0.7.2) + activejob (= 5.0.7.2) + activemodel (= 5.0.7.2) + activerecord (= 5.0.7.2) + activesupport (= 5.0.7.2) bundler (>= 1.3.0) - railties (= 5.0.7.1) + railties (= 5.0.7.2) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.2) actionpack (~> 5.x, >= 5.0.1) @@ -662,9 +662,9 @@ GEM rails-i18n (5.1.1) i18n (>= 0.7, < 2) railties (>= 5.0, < 6) - railties (5.0.7.1) - actionpack (= 5.0.7.1) - activesupport (= 5.0.7.1) + railties (5.0.7.2) + actionpack (= 5.0.7.2) + activesupport (= 5.0.7.2) method_source rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) @@ -1110,7 +1110,7 @@ DEPENDENCIES rack-cors (~> 1.0.0) rack-oauth2 (~> 1.2.1) rack-proxy (~> 0.6.0) - rails (= 5.0.7.1) + rails (= 5.0.7.2) rails-controller-testing rails-deprecated_sanitizer (~> 1.0.3) rails-i18n (~> 5.1) diff --git a/VERSION b/VERSION index 9ccabb205f..25ac99ff4c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -11.8.6 +11.8.9 diff --git a/app/assets/javascripts/pipelines/pipeline_details_mediator.js b/app/assets/javascripts/pipelines/pipeline_details_mediator.js index bd1e189566..d67d88c4db 100644 --- a/app/assets/javascripts/pipelines/pipeline_details_mediator.js +++ b/app/assets/javascripts/pipelines/pipeline_details_mediator.js @@ -19,6 +19,7 @@ export default class pipelinesMediator { this.poll = new Poll({ resource: this.service, method: 'getPipeline', + data: this.store.state.expandedPipelines ? this.getExpandedParameters() : undefined, successCallback: this.successCallback.bind(this), errorCallback: this.errorCallback.bind(this), }); @@ -56,6 +57,19 @@ export default class pipelinesMediator { .getPipeline() .then(response => this.successCallback(response)) .catch(() => this.errorCallback()) - .finally(() => this.poll.restart()); + .finally(() => + this.poll.restart( + this.store.state.expandedPipelines ? this.getExpandedParameters() : undefined, + ), + ); + } + + /** + * Backend expects paramets in the following format: `expanded[]=id&expanded[]=id` + */ + getExpandedParameters() { + return { + expanded: this.store.state.expandedPipelines, + }; } } diff --git a/app/assets/javascripts/pipelines/services/pipeline_service.js b/app/assets/javascripts/pipelines/services/pipeline_service.js index a53a9cc836..e44eb9cdfd 100644 --- a/app/assets/javascripts/pipelines/services/pipeline_service.js +++ b/app/assets/javascripts/pipelines/services/pipeline_service.js @@ -5,8 +5,8 @@ export default class PipelineService { this.pipeline = endpoint; } - getPipeline() { - return axios.get(this.pipeline); + getPipeline(params) { + return axios.get(this.pipeline, { params }); } // eslint-disable-next-line class-methods-use-this diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index 80b9bdc8f2..abc57b6ddc 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -78,7 +78,7 @@ module NotesActions # rubocop:disable Gitlab/ModuleWithInstanceVariables def update - @note = Notes::UpdateService.new(project, current_user, note_params).execute(note) + @note = Notes::UpdateService.new(project, current_user, update_note_params).execute(note) prepare_notes_for_rendering([@note]) respond_to do |format| @@ -216,6 +216,10 @@ module NotesActions ) end + def update_note_params + params.require(:note).permit(:note) + end + def set_polling_interval_header Gitlab::PollingInterval.set_header(response, interval: 6_000) end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index e2879bfdcf..c5bab877c0 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -136,18 +136,9 @@ module TreeHelper end # returns the relative path of the first subdir that doesn't have only one directory descendant - # rubocop: disable CodeReuse/ActiveRecord def flatten_tree(root_path, tree) - return tree.flat_path.sub(%r{\A#{Regexp.escape(root_path)}/}, '') if tree.flat_path.present? - - subtree = Gitlab::Git::Tree.where(@repository, @commit.id, tree.path) - if subtree.count == 1 && subtree.first.dir? - return tree_join(tree.name, flatten_tree(root_path, subtree.first)) - else - return tree.name - end + tree.flat_path.sub(%r{\A#{Regexp.escape(root_path)}/}, '') end - # rubocop: enable CodeReuse/ActiveRecord def selected_branch @branch_name || tree_edit_branch diff --git a/app/models/application_record.rb b/app/models/application_record.rb index a3d662d825..289864bbaa 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -7,6 +7,19 @@ class ApplicationRecord < ActiveRecord::Base where(id: ids) end + def self.safe_ensure_unique(retries: 0) + transaction(requires_new: true) do + yield + end + rescue ActiveRecord::RecordNotUnique + if retries > 0 + retries -= 1 + retry + end + + false + end + def self.safe_find_or_create_by!(*args) safe_find_or_create_by(*args).tap do |record| record.validate! unless record.persisted? @@ -14,10 +27,8 @@ class ApplicationRecord < ActiveRecord::Base end def self.safe_find_or_create_by(*args) - transaction(requires_new: true) do + safe_ensure_unique(retries: 1) do find_or_create_by(*args) end - rescue ActiveRecord::RecordNotUnique - retry end end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 481be2da8a..be2f1319fd 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -301,6 +301,11 @@ class MergeRequestDiff < ActiveRecord::Base private + def encode_in_base64?(diff_text) + (diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only?) || + diff_text.include?("\0") + end + def create_merge_request_diff_files(diffs) rows = if has_attribute?(:external_diff) && Gitlab.config.external_diffs.enabled @@ -353,7 +358,7 @@ class MergeRequestDiff < ActiveRecord::Base diff_hash.tap do |hash| diff_text = hash[:diff] - if diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only? + if encode_in_base64?(diff_text) hash[:binary] = true hash[:diff] = [diff_text].pack('m0') end diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb index 9f16eefe07..8104f00d13 100644 --- a/app/models/notification_recipient.rb +++ b/app/models/notification_recipient.rb @@ -123,15 +123,19 @@ class NotificationRecipient return @read_ability if instance_variable_defined?(:@read_ability) @read_ability = - case @target - when Issuable - :"read_#{@target.to_ability_name}" - when Ci::Pipeline + if @target.is_a?(Ci::Pipeline) :read_build # We have build trace in pipeline emails - when ActiveRecord::Base - :"read_#{@target.class.model_name.name.underscore}" - else - nil + elsif default_ability_for_target + :"read_#{default_ability_for_target}" + end + end + + def default_ability_for_target + @default_ability_for_target ||= + if @target.respond_to?(:to_ability_name) + @target.to_ability_name + elsif @target.class.respond_to?(:model_name) + @target.class.model_name.name.underscore end end diff --git a/app/validators/sha_validator.rb b/app/validators/sha_validator.rb index 085fca4d65..77e7cfa4f6 100644 --- a/app/validators/sha_validator.rb +++ b/app/validators/sha_validator.rb @@ -2,7 +2,7 @@ class ShaValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - return if value.blank? || value.match(/\A\h{40}\z/) + return if value.blank? || Commit.valid_hash?(value) record.errors.add(attribute, 'is not a valid SHA') end diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index db23447dd3..78de5548da 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -3,7 +3,7 @@ #{link_to @merge_request.author_name, user_url(@merge_request.author)} created a merge request: %p.details - != merge_path_description(@merge_request, '→') + = merge_path_description(@merge_request, '→') - if @merge_request.assignee_id.present? %p diff --git a/app/views/projects/buttons/_clone.html.haml b/app/views/projects/buttons/_clone.html.haml index 159d9e44e1..09f05b3043 100644 --- a/app/views/projects/buttons/_clone.html.haml +++ b/app/views/projects/buttons/_clone.html.haml @@ -7,7 +7,7 @@ = sprite_icon("arrow-down", css_class: "icon") %ul.p-3.dropdown-menu.dropdown-menu-right.dropdown-menu-large.dropdown-menu-selectable.clone-options-dropdown.qa-clone-options - if ssh_enabled? - %li.pb-2 + %li %label.label-bold = _('Clone with SSH') .input-group @@ -16,7 +16,7 @@ = clipboard_button(target: '#ssh_project_clone', title: _("Copy URL to clipboard"), class: "input-group-text btn-default btn-clipboard") = render_if_exists 'projects/buttons/geo' - if http_enabled? - %li + %li.pt-2 %label.label-bold = _('Clone with %{http_label}') % { http_label: gitlab_config.protocol.upcase } .input-group @@ -24,5 +24,6 @@ .input-group-append = clipboard_button(target: '#http_project_clone', title: _("Copy URL to clipboard"), class: "input-group-text btn-default btn-clipboard") = render_if_exists 'projects/buttons/geo' + = render_if_exists 'projects/buttons/kerberos_clone_field' = render_if_exists 'shared/geo_info_modal', project: project diff --git a/app/views/shared/_mobile_clone_panel.html.haml b/app/views/shared/_mobile_clone_panel.html.haml index 6e2527bd1a..1e6b6f7c79 100644 --- a/app/views/shared/_mobile_clone_panel.html.haml +++ b/app/views/shared/_mobile_clone_panel.html.haml @@ -13,3 +13,4 @@ - if http_enabled? %li = dropdown_item_with_description(http_copy_label, project.http_url_to_repo, href: project.http_url_to_repo, data: { clone_type: 'http' }) + = render_if_exists 'shared/mobile_kerberos_clone' diff --git a/debian/changelog b/debian/changelog index 5c12360533..1679c20d67 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +gitlab (11.8.9+dfsg-1) unstable; urgency=medium + + * Team upload + * New upstream version 11.8.9 (Fixes: CVE-2019-11544, CVE-2019-11546, + CVE-2019-11547, CVE-2019-11548, CVE-2019-11549) (Closes: #928221) + * Update d/patches/* + + -- Utkarsh Gupta Sat, 04 May 2019 01:20:45 +0530 + gitlab (11.8.6+dfsg-1+fto10+1) buster-fasttrack; urgency=medium * Rebuild for buster-fasttrack @@ -205,7 +214,7 @@ gitlab (11.5.3+dfsg-1) experimental; urgency=medium gitlab (11.4.9+dfsg-2) unstable; urgency=medium - * Reupload to unstable + * Reupload to unstable -- Pirate Praveen Sat, 08 Dec 2018 12:49:53 +0530 @@ -522,7 +531,7 @@ gitlab (9.5.4+dfsg-3) experimental; urgency=medium gitlab (9.5.4+dfsg-2) experimental; urgency=medium * Relax dependencies in Gemfile - * Tighten dependencies in control + * Tighten dependencies in control * Bump standards * Add missing attributions * Fix permissions for .gitlab_shell_secret @@ -545,13 +554,13 @@ gitlab (9.5.4+dfsg-1) experimental; urgency=medium gitlab (8.13.11+dfsg1-11) unstable; urgency=medium - * Tighten dependency on ruby-truncato + * Tighten dependency on ruby-truncato -- Pirate Praveen Mon, 14 Aug 2017 12:21:40 +0530 gitlab (8.13.11+dfsg1-10) unstable; urgency=medium - * Relax dependency on ruby-net-ssh (Closes: #868246) + * Relax dependency on ruby-net-ssh (Closes: #868246) -- Pirate Praveen Sun, 30 Jul 2017 16:14:02 +0530 @@ -594,7 +603,7 @@ gitlab (8.13.11+dfsg1-5) unstable; urgency=medium gitlab (8.13.11+dfsg1-4) unstable; urgency=medium * Check if gitlab_data_dir is defined before using it - * Ask email address for letsencrypt updates + * Ask email address for letsencrypt updates -- Pirate Praveen Wed, 26 Apr 2017 21:12:25 +0530 @@ -650,7 +659,7 @@ gitlab (8.13.11+dfsg-7) unstable; urgency=medium gitlab (8.13.11+dfsg-6) unstable; urgency=medium - * Improve configuration file parsing by using source (Closes: #857967) + * Improve configuration file parsing by using source (Closes: #857967) -- Pirate Praveen Fri, 17 Mar 2017 22:29:40 +0530 @@ -692,7 +701,7 @@ gitlab (8.13.11+dfsg-2) unstable; urgency=medium * Use upstream patch for git 2.11 support (Closes: #853251) * Set minimum version of gitlab-shell to 3.6.6-3 - (required for git 2.11 support) + (required for git 2.11 support) -- Pirate Praveen Tue, 07 Feb 2017 11:24:36 +0530 @@ -700,7 +709,7 @@ gitlab (8.13.11+dfsg-1) unstable; urgency=medium * New upstream release * Remove WoSign from suggested free SSL providers (they stopped providing - free SSL certificates from September 2016) + free SSL certificates from September 2016) * Backport git 2.11 support (Closes: #851714) -- Pirate Praveen Wed, 18 Jan 2017 13:21:17 +0530 @@ -721,7 +730,7 @@ gitlab (8.13.6+dfsg2-1) unstable; urgency=medium * Run tests with RAILS_ENV=test * Relax dependency on ruby-grape-entity - * Remove tasks related to linting from orig.tar + * Remove tasks related to linting from orig.tar -- Pirate Praveen Wed, 30 Nov 2016 16:15:29 +0530 @@ -775,21 +784,21 @@ gitlab (8.12.3+dfsg1-1) unstable; urgency=medium gitlab (8.11.3+dfsg1-3) unstable; urgency=medium - * Run some of gitlab provided tests as autopkgtests + * Run some of gitlab provided tests as autopkgtests -- Pirate Praveen Sat, 17 Sep 2016 21:41:51 +0530 gitlab (8.11.3+dfsg1-2) unstable; urgency=medium * Use config/initializers/secret_token.rb to create secrets.yml - (backup your secrets.yml if you are upgrading) + (backup your secrets.yml if you are upgrading) -- Pirate Praveen Sat, 17 Sep 2016 14:53:04 +0530 gitlab (8.11.3+dfsg1-1) unstable; urgency=medium * New upstream release - * Remove ruby-devise-async dependency + * Remove ruby-devise-async dependency -- Pirate Praveen Fri, 16 Sep 2016 12:44:05 +0530 @@ -803,13 +812,13 @@ gitlab (8.10.5+dfsg-3) unstable; urgency=high gitlab (8.10.5+dfsg-2) unstable; urgency=medium - * Reupload to unstable + * Reupload to unstable -- Pirate Praveen Thu, 01 Sep 2016 13:17:03 +0530 gitlab (8.10.5+dfsg-1) experimental; urgency=medium - * New upstream release + * New upstream release rouge 2.0 is now compatible (Closes: #830111) * Refresh patches * Update dependencies @@ -818,7 +827,7 @@ gitlab (8.10.5+dfsg-1) experimental; urgency=medium gitlab (8.9.0+dfsg-10) unstable; urgency=medium - * Relax dependency on rails in Gemfile (Closes: #834907) + * Relax dependency on rails in Gemfile (Closes: #834907) -- Pirate Praveen Mon, 22 Aug 2016 11:07:21 +0530 @@ -829,7 +838,7 @@ gitlab (8.9.0+dfsg-9) unstable; urgency=medium [ Dmitry Smirnov ] * New patch to fix error 500 on runners page (Closes: #819903) Thanks, Libor Klepáč. - + -- Pirate Praveen Mon, 15 Aug 2016 19:51:05 +0530 gitlab (8.9.0+dfsg-8) unstable; urgency=medium @@ -842,13 +851,13 @@ gitlab (8.9.0+dfsg-8) unstable; urgency=medium gitlab (8.9.0+dfsg-7) unstable; urgency=medium * Tighten dependencies - * Allow unicorn 5.0 + * Allow unicorn 5.0 -- Pirate Praveen Thu, 21 Jul 2016 13:51:37 +0530 gitlab (8.9.0+dfsg-6) unstable; urgency=medium - * Create .ssh/authorized_keys in postinst + * Create .ssh/authorized_keys in postinst -- Pirate Praveen Wed, 20 Jul 2016 23:13:59 +0530 @@ -858,7 +867,7 @@ gitlab (8.9.0+dfsg-5) unstable; urgency=medium * Don't run gitlab:shell:install in postinst (Closes: #831877) (installed config.yml works) * Add a note about CAcert and browser trust - + [ Dmitry Smirnov ] * Do not leave dangling symlinks behind after purge * Remove generated assets on purge @@ -883,7 +892,7 @@ gitlab (8.9.0+dfsg-4) unstable; urgency=medium gitlab (8.9.0+dfsg-3) unstable; urgency=medium - * Relax grape and rouge in Gemfile + * Relax grape and rouge in Gemfile -- Pirate Praveen Sun, 10 Jul 2016 20:23:41 +0530 @@ -897,7 +906,7 @@ gitlab (8.9.0+dfsg-2) unstable; urgency=medium gitlab (8.9.0+dfsg-1) experimental; urgency=medium * New upstream release - * Refresh patches and update dependencies + * Refresh patches and update dependencies -- Pirate Praveen Thu, 23 Jun 2016 23:54:28 +0530 @@ -913,7 +922,7 @@ gitlab (8.9.0+dfsg~rc4-1) experimental; urgency=medium gitlab (8.8.2+dfsg-5) unstable; urgency=medium - * Relax dependencies for all stable libraries (>= 1.0) (Closes: #827374) + * Relax dependencies for all stable libraries (>= 1.0) (Closes: #827374) -- Pirate Praveen Thu, 16 Jun 2016 12:31:40 +0530 @@ -990,7 +999,7 @@ gitlab (8.5.8+dfsg-3) unstable; urgency=medium gitlab (8.5.8+dfsg-2) experimental; urgency=medium - * Change letsencrypt from depends to recommends + * Change letsencrypt from depends to recommends -- Pirate Praveen Sun, 03 Apr 2016 11:09:51 +0530 @@ -1052,7 +1061,7 @@ gitlab (8.4.3+dfsg-8) unstable; urgency=medium * Install tmpfiles.d/gitlab.conf and allow www-data user to read /run/gitlab (Closes: #814714) - * Switch to sendmail method by default. + * Switch to sendmail method by default. -- Pirate Praveen Tue, 16 Feb 2016 13:58:16 +0530 @@ -1067,13 +1076,13 @@ gitlab (8.4.3+dfsg-7) unstable; urgency=medium gitlab (8.4.3+dfsg-6) unstable; urgency=medium - * Fix pid and sockets path for gitlab-workhorse + * Fix pid and sockets path for gitlab-workhorse -- Pirate Praveen Sun, 14 Feb 2016 12:45:24 +0530 gitlab (8.4.3+dfsg-5) unstable; urgency=medium - * Switch to su from sudo + * Switch to su from sudo -- Pirate Praveen Sat, 13 Feb 2016 23:53:42 +0530 diff --git a/debian/copyright b/debian/copyright index 30c4db0bb2..e8a380c74a 100644 --- a/debian/copyright +++ b/debian/copyright @@ -2,6 +2,10 @@ Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ Upstream-Name: gitlab-ce Source: https://gitlab.com/gitlab-org/gitlab-ce Files-Excluded: + vendor/assets/javascripts/pdf.js + vendor/assets/javascripts/pdf.min.js + vendor/assets/javascripts/pdf.worker.js + vendor/assets/javascripts/pdf.worker.min.js Comment: This package installs front end dependencies (nodejs modules) using yarn package manager from outside debian. This can go to main when all those nodejs modules are packaged in main. diff --git a/debian/patches/0050-relax-stable-libs.patch b/debian/patches/0050-relax-stable-libs.patch index ada9de01c8..f36be82586 100644 --- a/debian/patches/0050-relax-stable-libs.patch +++ b/debian/patches/0050-relax-stable-libs.patch @@ -6,13 +6,13 @@ gitlab Gemfile @@ -1,7 +1,7 @@ source 'https://rubygems.org' - gem 'rails', '5.0.7.1' + gem 'rails', '5.0.7.2' -gem 'rails-deprecated_sanitizer', '~> 1.0.3' +gem 'rails-deprecated_sanitizer', '~> 1.0', '>= 1.0.3' # Improves copy-on-write performance for MRI gem 'nakayoshi_fork', '~> 0.0.4' -@@ -9,7 +9,7 @@ +@@ -9,7 +9,7 @@ gem 'nakayoshi_fork', '~> 0.0.4' # Responders respond_to and respond_with gem 'responders', '~> 2.0' @@ -21,7 +21,7 @@ gitlab Gemfile # Default values for AR models gem 'gitlab-default_value_for', '~> 3.1.1', require: 'default_value_for' -@@ -28,57 +28,57 @@ +@@ -28,57 +28,57 @@ gem 'devise', '~> 4.4' gem 'doorkeeper', '~> 4.3' gem 'doorkeeper-openid_connect', '~> 1.5' gem 'omniauth', '~> 1.8' @@ -97,7 +97,7 @@ gitlab Gemfile # Disable strong_params so that Mash does not respond to :permitted? gem 'hashie-forbidden_attributes' -@@ -87,16 +87,16 @@ +@@ -87,16 +87,16 @@ gem 'hashie-forbidden_attributes' gem 'kaminari', '~> 1.0' # HAML @@ -117,7 +117,7 @@ gitlab Gemfile gem 'fog-local', '~> 0.3' gem 'fog-openstack', '~> 0.1' gem 'fog-rackspace', '~> 0.1.1' -@@ -109,32 +109,32 @@ +@@ -109,32 +109,32 @@ gem 'google-api-client', '~> 0.23' gem 'unf', '~> 0.1.4' # Seed data @@ -159,7 +159,7 @@ gitlab Gemfile # Application server # The 2.0.6 version of rack requires monkeypatch to be present in -@@ -143,7 +143,7 @@ +@@ -143,7 +143,7 @@ gem 'diffy', '~> 3.1.0' gem 'rack', '2.0.6' group :unicorn do @@ -168,7 +168,7 @@ gitlab Gemfile gem 'unicorn-worker-killer', '~> 0.4.4' end -@@ -159,9 +159,9 @@ +@@ -159,9 +159,9 @@ gem 'state_machines-activerecord', '~> 0.5.1' gem 'acts-as-taggable-on', '~> 5.0' # Background jobs @@ -180,7 +180,7 @@ gitlab Gemfile gem 'gitlab-sidekiq-fetcher', '~> 0.4.0', require: 'sidekiq-reliable-fetch' # Cron Parser -@@ -177,14 +177,14 @@ +@@ -177,14 +177,14 @@ gem 'rainbow', '~> 3.0' gem 'ruby-progressbar' # GitLab settings @@ -198,7 +198,7 @@ gitlab Gemfile # Export Ruby Regex to Javascript gem 'js_regex', '~> 3.1' -@@ -193,7 +193,7 @@ +@@ -193,7 +193,7 @@ gem 'js_regex', '~> 3.1' gem 'device_detector' # Cache @@ -207,7 +207,7 @@ gitlab Gemfile # Redis gem 'redis', '~> 3.2' -@@ -203,7 +203,7 @@ +@@ -203,7 +203,7 @@ gem 'connection_pool', '~> 2.0' gem 'discordrb-webhooks-blackst0ne', '~> 3.3', require: false # HipChat integration @@ -216,7 +216,7 @@ gitlab Gemfile # JIRA integration gem 'jira-ruby', '~> 1.4' -@@ -212,7 +212,7 @@ +@@ -212,7 +212,7 @@ gem 'jira-ruby', '~> 1.4' gem 'flowdock', '~> 0.7' # Slack integration @@ -225,7 +225,7 @@ gitlab Gemfile # Hangouts Chat integration gem 'hangouts-chat', '~> 0.0.5' -@@ -224,11 +224,11 @@ +@@ -224,11 +224,11 @@ gem 'asana', '~> 0.8.1' gem 'ruby-fogbugz', '~> 0.2.1' # Kubernetes integration @@ -239,7 +239,7 @@ gitlab Gemfile # Sanitizes SVG input gem 'loofah', '~> 2.2' -@@ -237,10 +237,10 @@ +@@ -237,10 +237,10 @@ gem 'loofah', '~> 2.2' gem 'licensee', '~> 8.9' # Protect against bruteforcing @@ -252,7 +252,7 @@ gitlab Gemfile # Detect and convert string character encoding gem 'charlock_holmes', '~> 0.7.5' -@@ -258,41 +258,41 @@ +@@ -258,41 +258,41 @@ gem 'chronic_duration', '~> 0.10.6' gem 'webpack-rails', '~> 0.9.10' gem 'rack-proxy', '~> 0.6.0' @@ -300,7 +300,7 @@ gitlab Gemfile gem 'peek-gc', '~> 0.0.2' -gem 'peek-mysql2', '~> 1.2.0', group: :mysql -gem 'peek-pg', '~> 1.3.0', group: :postgres -+gem 'peek-mysql2', '~> 1.2', group: :mysql ++gem 'peek-mysql2', '~> 1.2', group: :mysq +gem 'peek-pg', '~> 1.3', group: :postgres gem 'peek-rblineprof', '~> 0.2.0' -gem 'peek-redis', '~> 1.2.0' @@ -308,7 +308,7 @@ gitlab Gemfile # Metrics group :metrics do -@@ -318,54 +318,54 @@ +@@ -318,54 +318,54 @@ group :development do gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false # Better errors handler @@ -378,7 +378,7 @@ gitlab Gemfile gem 'license_finder', '~> 5.4', require: false gem 'knapsack', '~> 1.17' -@@ -374,18 +374,18 @@ +@@ -374,18 +374,18 @@ group :development, :test do gem 'stackprof', '~> 0.2.10', require: false @@ -403,7 +403,7 @@ gitlab Gemfile gem 'concurrent-ruby', '~> 1.1' gem 'test-prof', '~> 0.2.5' gem 'rspec_junit_formatter' -@@ -405,15 +405,15 @@ +@@ -405,15 +405,15 @@ gem 'rbtrace', '~> 0.4', require: false gem 'oauth2', '~> 1.4' # Health check @@ -423,7 +423,7 @@ gitlab Gemfile # Required for ED25519 SSH host key support group :ed25519 do -@@ -422,12 +422,12 @@ +@@ -422,12 +422,12 @@ group :ed25519 do end # Gitaly GRPC client diff --git a/debian/patches/0290-skip-peek-mysql2.patch b/debian/patches/0290-skip-peek-mysql2.patch index 60c5317457..584793bea2 100644 --- a/debian/patches/0290-skip-peek-mysql2.patch +++ b/debian/patches/0290-skip-peek-mysql2.patch @@ -1,10 +1,10 @@ --- a/Gemfile +++ b/Gemfile -@@ -289,7 +289,6 @@ +@@ -289,7 +289,6 @@ gem 'batch-loader', '~> 1.2', '>= 1.2.2' # Perf bar gem 'peek', '~> 1.0', '>= 1.0.1' gem 'peek-gc', '~> 0.0.2' --gem 'peek-mysql2', '~> 1.2', group: :mysql +-gem 'peek-mysql2', '~> 1.2', group: :mysq gem 'peek-pg', '~> 1.3', group: :postgres gem 'peek-rblineprof', '~> 0.2.0' gem 'peek-redis', '~> 1.2' diff --git a/debian/patches/0470-relax-rails.patch b/debian/patches/0470-relax-rails.patch index 7a45f7fe4e..e603d7b87d 100644 --- a/debian/patches/0470-relax-rails.patch +++ b/debian/patches/0470-relax-rails.patch @@ -3,7 +3,7 @@ @@ -1,6 +1,6 @@ source 'https://rubygems.org' --gem 'rails', '5.0.7.1' +-gem 'rails', '5.0.7.2' +gem 'rails', '~> 5.0' gem 'rails-deprecated_sanitizer', '~> 1.0', '>= 1.0.3' diff --git a/debian/patches/0680-rails-5_1.patch b/debian/patches/0680-rails-5_1.patch index ec4497f64f..1c65ca150b 100644 --- a/debian/patches/0680-rails-5_1.patch +++ b/debian/patches/0680-rails-5_1.patch @@ -70,71 +70,6 @@ Model.new.attributes now also returns encrypted attributes. --- a/Gemfile.lock +++ b/Gemfile.lock -@@ -4,41 +4,41 @@ - RedCloth (4.3.2) - abstract_type (0.0.7) - ace-rails-ap (4.1.2) -- actioncable (5.0.7.1) -- actionpack (= 5.0.7.1) -- nio4r (>= 1.2, < 3.0) -+ actioncable (5.1.6.1) -+ actionpack (= 5.1.6.1) -+ nio4r (~> 2.0) - websocket-driver (~> 0.6.1) -- actionmailer (5.0.7.1) -- actionpack (= 5.0.7.1) -- actionview (= 5.0.7.1) -- activejob (= 5.0.7.1) -+ actionmailer (5.1.6.1) -+ actionpack (= 5.1.6.1) -+ actionview (= 5.1.6.1) -+ activejob (= 5.1.6.1) - mail (~> 2.5, >= 2.5.4) - rails-dom-testing (~> 2.0) -- actionpack (5.0.7.1) -- actionview (= 5.0.7.1) -- activesupport (= 5.0.7.1) -+ actionpack (5.1.6.1) -+ actionview (= 5.1.6.1) -+ activesupport (= 5.1.6.1) - rack (~> 2.0) -- rack-test (~> 0.6.3) -+ rack-test (>= 0.6.3) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.0.2) -- actionview (5.0.7.1) -- activesupport (= 5.0.7.1) -+ actionview (5.1.6.1) -+ activesupport (= 5.1.6.1) - builder (~> 3.1) -- erubis (~> 2.7.0) -+ erubi (~> 1.4) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.0.3) -- activejob (5.0.7.1) -- activesupport (= 5.0.7.1) -+ activejob (5.1.6.1) -+ activesupport (= 5.1.6.1) - globalid (>= 0.3.6) -- activemodel (5.0.7.1) -- activesupport (= 5.0.7.1) -- activerecord (5.0.7.1) -- activemodel (= 5.0.7.1) -- activesupport (= 5.0.7.1) -- arel (~> 7.0) -+ activemodel (5.1.6.1) -+ activesupport (= 5.1.6.1) -+ activerecord (5.1.6.1) -+ activemodel (= 5.1.6.1) -+ activesupport (= 5.1.6.1) -+ arel (~> 8.0) - activerecord_sane_schema_dumper (1.0) - rails (>= 5, < 6) -- activesupport (5.0.7.1) -+ activesupport (5.1.6.1) - concurrent-ruby (~> 1.0, >= 1.0.2) - i18n (>= 0.7, < 2) - minitest (~> 5.1) @@ -52,7 +52,7 @@ public_suffix (>= 2.0.2, < 4.0) aes_key_wrap (1.0.1) @@ -174,60 +109,6 @@ Model.new.attributes now also returns encrypted attributes. grape (~> 1.0) rake (~> 12) grape_logging (1.7.0) -@@ -385,7 +384,7 @@ - json (~> 1.8) - multi_xml (>= 0.5.2) - httpclient (2.8.3) -- i18n (1.2.0) -+ i18n (1.3.0) - concurrent-ruby (~> 1.0) - icalendar (2.4.1) - ice_nine (0.11.2) -@@ -634,19 +633,19 @@ - rack - rack-proxy (0.6.0) - rack -- rack-test (0.6.3) -- rack (>= 1.0) -- rails (5.0.7.1) -- actioncable (= 5.0.7.1) -- actionmailer (= 5.0.7.1) -- actionpack (= 5.0.7.1) -- actionview (= 5.0.7.1) -- activejob (= 5.0.7.1) -- activemodel (= 5.0.7.1) -- activerecord (= 5.0.7.1) -- activesupport (= 5.0.7.1) -+ rack-test (1.1.0) -+ rack (>= 1.0, < 3) -+ rails (5.1.6.1) -+ actioncable (= 5.1.6.1) -+ actionmailer (= 5.1.6.1) -+ actionpack (= 5.1.6.1) -+ actionview (= 5.1.6.1) -+ activejob (= 5.1.6.1) -+ activemodel (= 5.1.6.1) -+ activerecord (= 5.1.6.1) -+ activesupport (= 5.1.6.1) - bundler (>= 1.3.0) -- railties (= 5.0.7.1) -+ railties (= 5.1.6.1) - sprockets-rails (>= 2.0.0) - rails-controller-testing (1.0.2) - actionpack (~> 5.x, >= 5.0.1) -@@ -662,9 +661,9 @@ - rails-i18n (5.1.1) - i18n (>= 0.7, < 2) - railties (>= 5.0, < 6) -- railties (5.0.7.1) -- actionpack (= 5.0.7.1) -- activesupport (= 5.0.7.1) -+ railties (5.1.6.1) -+ actionpack (= 5.1.6.1) -+ activesupport (= 5.1.6.1) - method_source - rake (>= 0.8.7) - thor (>= 0.18.1, < 2.0) @@ -1033,7 +1032,7 @@ gpgme (~> 2.0.18) grape (~> 1.1.0) @@ -237,15 +118,6 @@ Model.new.attributes now also returns encrypted attributes. grape_logging (~> 1.7) graphiql-rails (~> 1.4.10) graphql (~> 1.8.0) -@@ -1110,7 +1109,7 @@ - rack-cors (~> 1.0.0) - rack-oauth2 (~> 1.2.1) - rack-proxy (~> 0.6.0) -- rails (= 5.0.7.1) -+ rails (= 5.1.6.1) - rails-controller-testing - rails-deprecated_sanitizer (~> 1.0.3) - rails-i18n (~> 5.1) --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -146,7 +146,7 @@ @@ -369,7 +241,7 @@ Model.new.attributes now also returns encrypted attributes. --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb -@@ -699,7 +699,7 @@ +@@ -702,7 +702,7 @@ end def reload_diff_if_branch_changed diff --git a/debian/upstream-file-list b/debian/upstream-file-list index f7377740d7..8596b0f7d2 100644 --- a/debian/upstream-file-list +++ b/debian/upstream-file-list @@ -52,7 +52,6 @@ debian doc docker docker-compose.yml -ee fixtures generator_templates jest.config.js diff --git a/doc/administration/high_availability/nfs.md b/doc/administration/high_availability/nfs.md index 74b0e2c818..78ebf8a083 100644 --- a/doc/administration/high_availability/nfs.md +++ b/doc/administration/high_availability/nfs.md @@ -37,6 +37,28 @@ options: circumstances it could lead to data loss if a failure occurs before data has synced. +### Improving NFS performance with GitLab + +If you are using NFS to share Git data, we recommend that you enable a +number of feature flags that will allow GitLab application processes to +access Git data directly instead of going through the [Gitaly +service](../gitaly/index.md). Depending on your workload and disk +performance, these flags may help improve performance. See [the +issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/57317) for more +details. + +To do this, run the Rake task: + +```sh +gitlab-rake gitlab:features:enable_rugged +``` + +If you need to undo this setting for some reason, run: + +```sh +gitlab-rake gitlab:features:disable_rugged +``` + ### Known issues On some customer systems, we have seen NFS clients slow precipitously due to diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index d5fc403bf8..d793528d9b 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -56,6 +56,46 @@ If your test-suite is failing with Gitaly issues, as a first step, try running: rm -rf tmp/tests/gitaly ``` +## Legacy Rugged code + +While Gitaly can handle all Git access, many of GitLab customers still +run Gitaly atop NFS. The legacy Rugged implementation for Git calls may +be faster than the Gitaly RPC due to N+1 Gitaly calls and other +reasons. See [the +issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/57317) for more +details. + +Until GitLab has eliminated most of these inefficiencies or the use of +NFS is discontinued for Git data, Rugged implementations of some of the +most commonly-used RPCs can be enabled via feature flags: + +* `rugged_find_commit` +* `rugged_get_tree_entries` +* `rugged_tree_entry` +* `rugged_commit_is_ancestor` +* `rugged_commit_tree_entry` +* `rugged_list_commits_by_oid` + +A convenience Rake task can be used to enable or disable these flags +all together. To enable: + +```sh +bundle exec rake gitlab:features:enable_rugged +``` + +To disable: + +```sh +bundle exec rake gitlab:features:disable_rugged +``` + +Most of this code exists in the `lib/gitlab/git/rugged_impl` directory. + +NOTE: **Note:** You should NOT need to add or modify code related to +Rugged unless explicitly discussed with the [Gitaly +Team](https://gitlab.com/groups/gl-gitaly/group_members). This code will +NOT work on GitLab.com or other GitLab instances that do not use NFS. + ## `TooManyInvocationsError` errors During development and testing, you may experience `Gitlab::GitalyClient::TooManyInvocationsError` failures. diff --git a/ee/changelogs/unreleased/security-milestone-labels.yml b/ee/changelogs/unreleased/security-milestone-labels.yml deleted file mode 100644 index 4f8abcbc8b..0000000000 --- a/ee/changelogs/unreleased/security-milestone-labels.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Check label_ids parent when updating issue board -merge_request: -author: -type: security diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index 259a2b7911..10df4ed72d 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -23,6 +23,10 @@ module Gitlab class << self def find(repository, sha, path, limit: MAX_DATA_DISPLAY_SIZE) + tree_entry(repository, sha, path, limit) + end + + def tree_entry(repository, sha, path, limit) return unless path path = path.sub(%r{\A/*}, '') @@ -179,3 +183,5 @@ module Gitlab end end end + +Gitlab::Git::Blob.singleton_class.prepend Gitlab::Git::RuggedImpl::Blob::ClassMethods diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 5863815ca8..7be8539f38 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -5,6 +5,7 @@ module Gitlab module Git class Commit include Gitlab::EncodingHelper + prepend Gitlab::Git::RuggedImpl::Commit extend Gitlab::Git::WrapsGitalyErrors attr_accessor :raw_commit, :head @@ -62,15 +63,19 @@ module Gitlab # This saves us an RPC round trip. return nil if commit_id.include?(':') - commit = wrapped_gitaly_errors do - repo.gitaly_commit_client.find_commit(commit_id) - end + commit = find_commit(repo, commit_id) decorate(repo, commit) if commit rescue Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository, ArgumentError nil end + def find_commit(repo, commit_id) + wrapped_gitaly_errors do + repo.gitaly_commit_client.find_commit(commit_id) + end + end + # Get last commit for HEAD # # Ex. @@ -185,6 +190,10 @@ module Gitlab @repository = repository @head = head + init_commit(raw_commit) + end + + def init_commit(raw_commit) case raw_commit when Hash init_from_hash(raw_commit) @@ -305,11 +314,16 @@ module Gitlab def tree_entry(path) return unless path.present? + commit_tree_entry(path) + end + + def commit_tree_entry(path) # We're only interested in metadata, so limit actual data to 1 byte # since Gitaly doesn't support "send no data" option. entry = @repository.gitaly_commit_client.tree_entry(id, path, 1) return unless entry + # To be compatible with the rugged format entry = entry.to_h entry.delete(:data) entry[:name] = File.basename(path) @@ -400,3 +414,5 @@ module Gitlab end end end + +Gitlab::Git::Commit.singleton_class.prepend Gitlab::Git::RuggedImpl::Commit::ClassMethods diff --git a/lib/gitlab/git/ref.rb b/lib/gitlab/git/ref.rb index eec9119494..47cfb48350 100644 --- a/lib/gitlab/git/ref.rb +++ b/lib/gitlab/git/ref.rb @@ -4,6 +4,7 @@ module Gitlab module Git class Ref include Gitlab::EncodingHelper + include Gitlab::Git::RuggedImpl::Ref # Branch or tag name # without "refs/tags|heads" prefix diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 54bbd53139..7516afb5ec 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -11,6 +11,7 @@ module Gitlab include Gitlab::Git::WrapsGitalyErrors include Gitlab::EncodingHelper include Gitlab::Utils::StrongMemoize + prepend Gitlab::Git::RuggedImpl::Repository SEARCH_CONTEXT_LINES = 3 REV_LIST_COMMIT_LIMIT = 2_000 diff --git a/lib/gitlab/git/rugged_impl/blob.rb b/lib/gitlab/git/rugged_impl/blob.rb new file mode 100644 index 0000000000..11ee4ebda4 --- /dev/null +++ b/lib/gitlab/git/rugged_impl/blob.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +# NOTE: This code is legacy. Do not add/modify code here unless you have +# discussed with the Gitaly team. See +# https://docs.gitlab.com/ee/development/gitaly.html#legacy-rugged-code +# for more details. + +module Gitlab + module Git + module RuggedImpl + module Blob + module ClassMethods + extend ::Gitlab::Utils::Override + + override :tree_entry + def tree_entry(repository, sha, path, limit) + if Feature.enabled?(:rugged_tree_entry) + rugged_tree_entry(repository, sha, path, limit) + else + super + end + end + + private + + def rugged_tree_entry(repository, sha, path, limit) + return unless path + + # Strip any leading / characters from the path + path = path.sub(%r{\A/*}, '') + + rugged_commit = repository.lookup(sha) + root_tree = rugged_commit.tree + + blob_entry = find_entry_by_path(repository, root_tree.oid, *path.split('/')) + + return unless blob_entry + + if blob_entry[:type] == :commit + submodule_blob(blob_entry, path, sha) + else + blob = repository.lookup(blob_entry[:oid]) + + if blob + new( + id: blob.oid, + name: blob_entry[:name], + size: blob.size, + # Rugged::Blob#content is expensive; don't call it if we don't have to. + data: limit.zero? ? '' : blob.content(limit), + mode: blob_entry[:filemode].to_s(8), + path: path, + commit_id: sha, + binary: blob.binary? + ) + end + end + rescue Rugged::ReferenceError + nil + end + + # Recursive search of blob id by path + # + # Ex. + # blog/ # oid: 1a + # app/ # oid: 2a + # models/ # oid: 3a + # file.rb # oid: 4a + # + # + # Blob.find_entry_by_path(repo, '1a', 'blog', 'app', 'file.rb') # => '4a' + # + def find_entry_by_path(repository, root_id, *path_parts) + root_tree = repository.lookup(root_id) + + entry = root_tree.find do |entry| + entry[:name] == path_parts[0] + end + + return unless entry + + if path_parts.size > 1 + return unless entry[:type] == :tree + + path_parts.shift + find_entry_by_path(repository, entry[:oid], *path_parts) + else + [:blob, :commit].include?(entry[:type]) ? entry : nil + end + end + + def submodule_blob(blob_entry, path, sha) + new( + id: blob_entry[:oid], + name: blob_entry[:name], + size: 0, + data: '', + path: path, + commit_id: sha + ) + end + end + end + end + end +end diff --git a/lib/gitlab/git/rugged_impl/commit.rb b/lib/gitlab/git/rugged_impl/commit.rb new file mode 100644 index 0000000000..bce4fa14fb --- /dev/null +++ b/lib/gitlab/git/rugged_impl/commit.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +# NOTE: This code is legacy. Do not add/modify code here unless you have +# discussed with the Gitaly team. See +# https://docs.gitlab.com/ee/development/gitaly.html#legacy-rugged-code +# for more details. + +# rubocop:disable Gitlab/ModuleWithInstanceVariables +module Gitlab + module Git + module RuggedImpl + module Commit + module ClassMethods + extend ::Gitlab::Utils::Override + + def rugged_find(repo, commit_id) + obj = repo.rev_parse_target(commit_id) + + obj.is_a?(::Rugged::Commit) ? obj : nil + rescue ::Rugged::Error + nil + end + + # This needs to return an array of Gitlab::Git:Commit objects + # instead of Rugged::Commit objects to ensure upstream models + # operate on a consistent interface. Unlike + # Gitlab::Git::Commit.find, Gitlab::Git::Commit.batch_by_oid + # doesn't attempt to decorate the result. + def rugged_batch_by_oid(repo, oids) + oids.map { |oid| rugged_find(repo, oid) } + .compact + .map { |commit| decorate(repo, commit) } + end + + override :find_commit + def find_commit(repo, commit_id) + if Feature.enabled?(:rugged_find_commit) + rugged_find(repo, commit_id) + else + super + end + end + + override :batch_by_oid + def batch_by_oid(repo, oids) + if Feature.enabled?(:rugged_list_commits_by_oid) + rugged_batch_by_oid(repo, oids) + else + super + end + end + end + + extend ::Gitlab::Utils::Override + + override :init_commit + def init_commit(raw_commit) + case raw_commit + when ::Rugged::Commit + init_from_rugged(raw_commit) + else + super + end + end + + override :commit_tree_entry + def commit_tree_entry(path) + if Feature.enabled?(:rugged_commit_tree_entry) + rugged_tree_entry(path) + else + super + end + end + + # Is this the same as Blob.find_entry_by_path ? + def rugged_tree_entry(path) + rugged_commit.tree.path(path) + rescue Rugged::TreeError + nil + end + + def rugged_commit + @rugged_commit ||= if raw_commit.is_a?(Rugged::Commit) + raw_commit + else + @repository.rev_parse_target(id) + end + end + + def init_from_rugged(commit) + author = commit.author + committer = commit.committer + + @raw_commit = commit + @id = commit.oid + @message = commit.message + @authored_date = author[:time] + @committed_date = committer[:time] + @author_name = author[:name] + @author_email = author[:email] + @committer_name = committer[:name] + @committer_email = committer[:email] + @parent_ids = commit.parents.map(&:oid) + end + end + end + end +end +# rubocop:enable Gitlab/ModuleWithInstanceVariables diff --git a/lib/gitlab/git/rugged_impl/ref.rb b/lib/gitlab/git/rugged_impl/ref.rb new file mode 100644 index 0000000000..b553e82dc4 --- /dev/null +++ b/lib/gitlab/git/rugged_impl/ref.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# NOTE: This code is legacy. Do not add/modify code here unless you have +# discussed with the Gitaly team. See +# https://docs.gitlab.com/ee/development/gitaly.html#legacy-rugged-code +# for more details. + +module Gitlab + module Git + module RuggedImpl + module Ref + def self.dereference_object(object) + object = object.target while object.is_a?(::Rugged::Tag::Annotation) + + object + end + end + end + end +end diff --git a/lib/gitlab/git/rugged_impl/repository.rb b/lib/gitlab/git/rugged_impl/repository.rb new file mode 100644 index 0000000000..e91b0ddcd3 --- /dev/null +++ b/lib/gitlab/git/rugged_impl/repository.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +# NOTE: This code is legacy. Do not add/modify code here unless you have +# discussed with the Gitaly team. See +# https://docs.gitlab.com/ee/development/gitaly.html#legacy-rugged-code +# for more details. + +# rubocop:disable Gitlab/ModuleWithInstanceVariables +module Gitlab + module Git + module RuggedImpl + module Repository + extend ::Gitlab::Utils::Override + + FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor rugged_commit_tree_entry rugged_list_commits_by_oid).freeze + + def alternate_object_directories + relative_object_directories.map { |d| File.join(path, d) } + end + + ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES = %w[ + GIT_OBJECT_DIRECTORY_RELATIVE + GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE + ].freeze + + def relative_object_directories + Gitlab::Git::HookEnv.all(gl_repository).values_at(*ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES).flatten.compact + end + + def rugged + @rugged ||= ::Rugged::Repository.new(path, alternates: alternate_object_directories) + rescue ::Rugged::RepositoryError, ::Rugged::OSError + raise ::Gitlab::Git::Repository::NoRepository.new('no repository for such path') + end + + def cleanup + @rugged&.close + end + + # Return the object that +revspec+ points to. If +revspec+ is an + # annotated tag, then return the tag's target instead. + def rev_parse_target(revspec) + obj = rugged.rev_parse(revspec) + Ref.dereference_object(obj) + end + + override :ancestor? + def ancestor?(from, to) + if Feature.enabled?(:rugged_commit_is_ancestor) + rugged_is_ancestor?(from, to) + else + super + end + end + + def rugged_is_ancestor?(ancestor_id, descendant_id) + return false if ancestor_id.nil? || descendant_id.nil? + + rugged_merge_base(ancestor_id, descendant_id) == ancestor_id + rescue Rugged::OdbError + false + end + + def rugged_merge_base(from, to) + rugged.merge_base(from, to) + rescue Rugged::ReferenceError + nil + end + + # Lookup for rugged object by oid or ref name + def lookup(oid_or_ref_name) + rugged.rev_parse(oid_or_ref_name) + end + end + end + end +end +# rubocop:enable Gitlab/ModuleWithInstanceVariables diff --git a/lib/gitlab/git/rugged_impl/tree.rb b/lib/gitlab/git/rugged_impl/tree.rb new file mode 100644 index 0000000000..bb13d114d4 --- /dev/null +++ b/lib/gitlab/git/rugged_impl/tree.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +# NOTE: This code is legacy. Do not add/modify code here unless you have +# discussed with the Gitaly team. See +# https://docs.gitlab.com/ee/development/gitaly.html#legacy-rugged-code +# for more details. + +module Gitlab + module Git + module RuggedImpl + module Tree + module ClassMethods + extend ::Gitlab::Utils::Override + + override :tree_entries + def tree_entries(repository, sha, path, recursive) + if Feature.enabled?(:rugged_tree_entries) + tree_entries_with_flat_path_from_rugged(repository, sha, path, recursive) + else + super + end + end + + def tree_entries_with_flat_path_from_rugged(repository, sha, path, recursive) + tree_entries_from_rugged(repository, sha, path, recursive).tap do |entries| + # This was an optimization to reduce N+1 queries for Gitaly + # (https://gitlab.com/gitlab-org/gitaly/issues/530). It + # used to be done lazily in the view via + # TreeHelper#flatten_tree, so it's possible there's a + # performance impact by loading this eagerly. + rugged_populate_flat_path(repository, sha, path, entries) + end + end + + def tree_entries_from_rugged(repository, sha, path, recursive) + current_path_entries = get_tree_entries_from_rugged(repository, sha, path) + ordered_entries = [] + + current_path_entries.each do |entry| + ordered_entries << entry + + if recursive && entry.dir? + ordered_entries.concat(tree_entries_from_rugged(repository, sha, entry.path, true)) + end + end + end + + def rugged_populate_flat_path(repository, sha, path, entries) + entries.each do |entry| + entry.flat_path = entry.path + + next unless entry.dir? + + entry.flat_path = + if path + File.join(path, rugged_flatten_tree(repository, sha, entry, path)) + else + rugged_flatten_tree(repository, sha, entry, path) + end + end + end + + # Returns the relative path of the first subdir that doesn't have only one directory descendant + def rugged_flatten_tree(repository, sha, tree, root_path) + subtree = tree_entries_from_rugged(repository, sha, tree.path, false) + + if subtree.count == 1 && subtree.first.dir? + File.join(tree.name, rugged_flatten_tree(repository, sha, subtree.first, root_path)) + else + tree.name + end + end + + def get_tree_entries_from_rugged(repository, sha, path) + commit = repository.lookup(sha) + root_tree = commit.tree + + tree = if path + id = find_id_by_path(repository, root_tree.oid, path) + if id + repository.lookup(id) + else + [] + end + else + root_tree + end + + tree.map do |entry| + current_path = path ? File.join(path, entry[:name]) : entry[:name] + + new( + id: entry[:oid], + root_id: root_tree.oid, + name: entry[:name], + type: entry[:type], + mode: entry[:filemode].to_s(8), + path: current_path, + commit_id: sha + ) + end + rescue Rugged::ReferenceError + [] + end + end + end + end + end +end diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb index 51542bcaaa..33505372ea 100644 --- a/lib/gitlab/git/tree.rb +++ b/lib/gitlab/git/tree.rb @@ -18,6 +18,10 @@ module Gitlab def where(repository, sha, path = nil, recursive = false) path = nil if path == '' || path == '/' + tree_entries(repository, sha, path, recursive) + end + + def tree_entries(repository, sha, path, recursive) wrapped_gitaly_errors do repository.gitaly_commit_client.tree_entries(repository, sha, path, recursive) end @@ -95,3 +99,5 @@ module Gitlab end end end + +Gitlab::Git::Tree.singleton_class.prepend Gitlab::Git::RuggedImpl::Tree::ClassMethods diff --git a/lib/gitlab/gitaly_client/storage_settings.rb b/lib/gitlab/gitaly_client/storage_settings.rb index 754cccb6b3..78ef6bfc0e 100644 --- a/lib/gitlab/gitaly_client/storage_settings.rb +++ b/lib/gitlab/gitaly_client/storage_settings.rb @@ -32,11 +32,19 @@ module Gitlab end def self.disk_access_denied? + return false if rugged_enabled? + !temporarily_allowed?(ALLOW_KEY) && GitalyClient.feature_enabled?(DISK_ACCESS_DENIED_FLAG) rescue false # Err on the side of caution, don't break gitlab for people end + def self.rugged_enabled? + Gitlab::Git::RuggedImpl::Repository::FEATURE_FLAGS.any? do |flag| + Feature.enabled?(flag) + end + end + def initialize(storage) raise InvalidConfigurationError, "expected a Hash, got a #{storage.class.name}" unless storage.is_a?(Hash) raise InvalidConfigurationError, INVALID_STORAGE_MESSAGE unless storage.has_key?('path') diff --git a/lib/gitlab/middleware/basic_health_check.rb b/lib/gitlab/middleware/basic_health_check.rb index acf8c301b8..84e4980542 100644 --- a/lib/gitlab/middleware/basic_health_check.rb +++ b/lib/gitlab/middleware/basic_health_check.rb @@ -24,7 +24,13 @@ module Gitlab def call(env) return @app.call(env) unless env['PATH_INFO'] == HEALTH_PATH - request = ActionDispatch::Request.new(env) + # We should be using ActionDispatch::Request instead of + # Rack::Request to be consistent with Rails, but due to a Rails + # bug described in + # https://gitlab.com/gitlab-org/gitlab-ce/issues/58573#note_149799010 + # hosts behind a load balancer will only see 127.0.0.1 for the + # load balancer's IP. + request = Rack::Request.new(env) return OK_RESPONSE if client_ip_whitelisted?(request) diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb index d9811e036d..f6d289476c 100644 --- a/lib/gitlab/request_context.rb +++ b/lib/gitlab/request_context.rb @@ -13,7 +13,13 @@ module Gitlab end def call(env) - req = ActionDispatch::Request.new(env) + # We should be using ActionDispatch::Request instead of + # Rack::Request to be consistent with Rails, but due to a Rails + # bug described in + # https://gitlab.com/gitlab-org/gitlab-ce/issues/58573#note_149799010 + # hosts behind a load balancer will only see 127.0.0.1 for the + # load balancer's IP. + req = Rack::Request.new(env) Gitlab::SafeRequestStore[:client_ip] = req.ip diff --git a/lib/tasks/gitlab/features.rake b/lib/tasks/gitlab/features.rake new file mode 100644 index 0000000000..d115961108 --- /dev/null +++ b/lib/tasks/gitlab/features.rake @@ -0,0 +1,24 @@ +namespace :gitlab do + namespace :features do + desc 'GitLab | Features | Enable direct Git access via Rugged for NFS' + task enable_rugged: :environment do + set_rugged_feature_flags(true) + puts 'All Rugged feature flags were enabled.' + end + + task disable_rugged: :environment do + set_rugged_feature_flags(false) + puts 'All Rugged feature flags were disabled.' + end + end + + def set_rugged_feature_flags(status) + Gitlab::Git::RuggedImpl::Repository::FEATURE_FLAGS.each do |flag| + if status + Feature.enable(flag) + else + Feature.disable(flag) + end + end + end +end diff --git a/scripts/lint-rugged b/scripts/lint-rugged index 22e3e1f150..9466c62a41 100755 --- a/scripts/lint-rugged +++ b/scripts/lint-rugged @@ -5,12 +5,18 @@ ALLOWED = [ 'lib/gitlab/bare_repository_import/repository.rb', # Needed to avoid using the git binary to validate a branch name - 'lib/gitlab/git_ref_validator.rb' + 'lib/gitlab/git_ref_validator.rb', + + # Reverted Rugged calls due to Gitaly atop NFS performance + # See https://docs.gitlab.com/ee/development/gitaly.html#legacy-rugged-code. + 'lib/gitlab/git/rugged_impl/', + 'lib/gitlab/gitaly_client/storage_settings.rb' ].freeze rugged_lines = IO.popen(%w[git grep -i -n rugged -- app config lib], &:read).lines rugged_lines = rugged_lines.select { |l| /^[^:]*\.rb:/ =~ l } rugged_lines = rugged_lines.reject { |l| l.start_with?(*ALLOWED) } +rugged_lines = rugged_lines.reject { |l| /(include|prepend) Gitlab::Git::RuggedImpl/ =~ l} rugged_lines = rugged_lines.reject do |line| code, _comment = line.split('# ', 2) code !~ /rugged/i diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index ec91a76038..5ea4d0b795 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -431,28 +431,77 @@ describe Projects::NotesController do end describe 'PUT update' do - context "should update the note with a valid issue" do - let(:request_params) do - { - namespace_id: project.namespace, - project_id: project, - id: note, - format: :json, - note: { - note: "New comment" + context "updates the note" do + context 'with a valid issue' do + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + id: note, + format: :json, + note: { + note: "New comment" + } } - } + end + + before do + sign_in(note.author) + project.add_developer(note.author) + end + + it "updates the note content" do + expect { put :update, params: request_params }.to change { note.reload.note } + end end - before do - sign_in(note.author) - project.add_developer(note.author) - end + context "when the note is edited and a different issue is targeted" do + ## + # We are editing a note originally in a public issue of a public project, + # but the edit can be intercepted to change the target to a different, even confidential, issue + # see https://gitlab.com/gitlab-org/gitlab-ce/issues/57153 + ## - it "updates the note" do - expect { put :update, params: request_params }.to change { note.reload.note } + let!(:confidential_issue) { create(:issue, :confidential, project: project) } + let(:new_content) { "splendiferous new content" } + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + id: note, + format: :json, + note: { + note: new_content, + noteable_id: confidential_issue.id + } + } + end + + before do + sign_in(note.author) + project.add_developer(note.author) + + put :update, params: request_params + end + + it 'returns success' do + expect(response.status).to eq 200 + end + + it 'edits the note content' do + expect(note.reload.note).to eq new_content + end + + it 'does not create a note in the confidential issue' do + expect(confidential_issue.reload.notes).to be_empty + end + + it "does not modify the note's issue" do + expect(note.noteable_id).to match note.reload.noteable_id + end end end + context "doesnt update the note" do let(:issue) { create(:issue, :confidential, project: project) } let(:note) { create(:note, noteable: issue, project: project) } diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index 8a44ce5284..ee5d27355f 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -82,6 +82,12 @@ FactoryBot.define do end end + trait :with_job do + after(:build) do |pipeline, evaluator| + pipeline.builds << build(:ci_build, pipeline: pipeline, project: pipeline.project) + end + end + trait :auto_devops_source do config_source { Ci::Pipeline.config_sources[:auto_devops_source] } end diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index c432cc223b..e1a2bae5fe 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -95,6 +95,9 @@ describe Gitlab::BitbucketImport::Importer do subject { described_class.new(project) } describe '#import_pull_requests' do + let(:source_branch_sha) { sample.commits.last } + let(:target_branch_sha) { sample.commits.first } + before do allow(subject).to receive(:import_wiki) allow(subject).to receive(:import_issues) @@ -102,9 +105,9 @@ describe Gitlab::BitbucketImport::Importer do pull_request = instance_double( Bitbucket::Representation::PullRequest, iid: 10, - source_branch_sha: sample.commits.last, + source_branch_sha: source_branch_sha, source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch, - target_branch_sha: sample.commits.first, + target_branch_sha: target_branch_sha, target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, title: 'This is a title', description: 'This is a test pull request', @@ -162,6 +165,19 @@ describe Gitlab::BitbucketImport::Importer do expect(reply_note).to be_a(DiffNote) expect(reply_note.note).to eq(@reply.note) end + + context "when branches' sha is not found in the repository" do + let(:source_branch_sha) { 'a' * Commit::MIN_SHA_LENGTH } + let(:target_branch_sha) { 'b' * Commit::MIN_SHA_LENGTH } + + it 'uses the pull request sha references' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + merge_request_diff = MergeRequest.first.merge_request_diff + expect(merge_request_diff.head_commit_sha).to eq source_branch_sha + expect(merge_request_diff.start_commit_sha).to eq target_branch_sha + end + end end context 'issues statuses' do diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index a1b5cea88c..10bc82e24d 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -18,7 +18,7 @@ describe Gitlab::Git::Blob, :seed_helper do end end - describe '.find' do + shared_examples '.find' do context 'nil path' do let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, nil) } @@ -128,6 +128,20 @@ describe Gitlab::Git::Blob, :seed_helper do end end + describe '.find with Gitaly enabled' do + it_behaves_like '.find' + end + + describe '.find with Rugged enabled', :enable_rugged do + it 'calls out to the Rugged implementation' do + allow_any_instance_of(Rugged).to receive(:rev_parse).with(SeedRepo::Commit::ID).and_call_original + + described_class.find(repository, SeedRepo::Commit::ID, 'files/images/6049019_460s.jpg') + end + + it_behaves_like '.find' + end + describe '.raw' do let(:raw_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::RubyBlob::ID) } let(:bad_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::BigCommit::ID) } diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 2611ebed25..f544cc55d3 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -112,7 +112,7 @@ describe Gitlab::Git::Commit, :seed_helper do end context 'Class methods' do - describe '.find' do + shared_examples '.find' do it "should return first head commit if without params" do expect(described_class.last(repository).id).to eq( rugged_repo.head.target.oid @@ -154,6 +154,20 @@ describe Gitlab::Git::Commit, :seed_helper do end end + describe '.find with Gitaly enabled' do + it_should_behave_like '.find' + end + + describe '.find with Rugged enabled', :enable_rugged do + it 'calls out to the Rugged implementation' do + allow_any_instance_of(Rugged).to receive(:rev_parse).with(SeedRepo::Commit::ID).and_call_original + + described_class.find(repository, SeedRepo::Commit::ID) + end + + it_should_behave_like '.find' + end + describe '.last_for_path' do context 'no path' do subject { described_class.last_for_path(repository, 'master') } @@ -366,7 +380,32 @@ describe Gitlab::Git::Commit, :seed_helper do end end - describe '#batch_by_oid' do + shared_examples '.batch_by_oid' do + context 'with multiple OIDs' do + let(:oids) { [SeedRepo::Commit::ID, SeedRepo::FirstCommit::ID] } + + it 'returns multiple commits' do + commits = described_class.batch_by_oid(repository, oids) + + expect(commits.count).to eq(2) + expect(commits).to all( be_a(Gitlab::Git::Commit) ) + expect(commits.first.sha).to eq(SeedRepo::Commit::ID) + expect(commits.second.sha).to eq(SeedRepo::FirstCommit::ID) + end + end + + context 'when oids is empty' do + it 'returns empty commits' do + commits = described_class.batch_by_oid(repository, []) + + expect(commits.count).to eq(0) + end + end + end + + describe '.batch_by_oid with Gitaly enabled' do + it_should_behave_like '.batch_by_oid' + context 'when oids is empty' do it 'makes no Gitaly request' do expect(Gitlab::GitalyClient).not_to receive(:call) @@ -376,6 +415,16 @@ describe Gitlab::Git::Commit, :seed_helper do end end + describe '.batch_by_oid with Rugged enabled', :enable_rugged do + it_should_behave_like '.batch_by_oid' + + it 'calls out to the Rugged implementation' do + allow_any_instance_of(Rugged).to receive(:rev_parse).with(SeedRepo::Commit::ID).and_call_original + + described_class.batch_by_oid(repository, [SeedRepo::Commit::ID]) + end + end + shared_examples 'extracting commit signature' do context 'when the commit is signed' do let(:commit_id) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 4a4d69490a..7ad3cde97f 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Tree, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } - context :repo do + shared_examples :repo do let(:tree) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID) } it { expect(tree).to be_kind_of Array } @@ -12,6 +12,17 @@ describe Gitlab::Git::Tree, :seed_helper do it { expect(tree.select(&:file?).size).to eq(10) } it { expect(tree.select(&:submodule?).size).to eq(2) } + it 'returns an empty array when called with an invalid ref' do + expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([]) + end + + it 'returns a list of tree objects' do + entries = described_class.where(repository, SeedRepo::Commit::ID, 'files', true) + + expect(entries.count).to be >= 5 + expect(entries).to all(be_a(Gitlab::Git::Tree)) + end + describe '#dir?' do let(:dir) { tree.select(&:dir?).first } @@ -20,8 +31,8 @@ describe Gitlab::Git::Tree, :seed_helper do it { expect(dir.commit_id).to eq(SeedRepo::Commit::ID) } it { expect(dir.name).to eq('encoding') } it { expect(dir.path).to eq('encoding') } - it { expect(dir.flat_path).to eq('encoding') } it { expect(dir.mode).to eq('40000') } + it { expect(dir.flat_path).to eq('encoding') } context :subdir do let(:subdir) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID, 'files').first } @@ -44,6 +55,51 @@ describe Gitlab::Git::Tree, :seed_helper do it { expect(subdir_file.path).to eq('files/ruby/popen.rb') } it { expect(subdir_file.flat_path).to eq('files/ruby/popen.rb') } end + + context :flat_path do + let(:filename) { 'files/flat/path/correct/content.txt' } + let(:oid) { create_file(filename) } + let(:subdir_file) { Gitlab::Git::Tree.where(repository, oid, 'files/flat').first } + let(:repository_rugged) { Rugged::Repository.new(File.join(SEED_STORAGE_PATH, TEST_REPO_PATH)) } + + it { expect(subdir_file.flat_path).to eq('files/flat/path/correct') } + end + + def create_file(path) + oid = repository_rugged.write('test', :blob) + index = repository_rugged.index + index.add(path: filename, oid: oid, mode: 0100644) + + options = commit_options( + repository_rugged, + index, + repository_rugged.head.target, + 'HEAD', + 'Add new file') + + Rugged::Commit.create(repository_rugged, options) + end + + # Build the options hash that's passed to Rugged::Commit#create + def commit_options(repo, index, target, ref, message) + options = {} + options[:tree] = index.write_tree(repo) + options[:author] = { + email: "test@example.com", + name: "Test Author", + time: Time.gm(2014, "mar", 3, 20, 15, 1) + } + options[:committer] = { + email: "test@example.com", + name: "Test Author", + time: Time.gm(2014, "mar", 3, 20, 15, 1) + } + options[:message] ||= message + options[:parents] = repo.empty? ? [] : [target].compact + options[:update_ref] = ref + + options + end end describe '#file?' do @@ -79,9 +135,17 @@ describe Gitlab::Git::Tree, :seed_helper do end end - describe '#where' do - it 'returns an empty array when called with an invalid ref' do - expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([]) + describe '.where with Gitaly enabled' do + it_behaves_like :repo + end + + describe '.where with Rugged enabled', :enable_rugged do + it 'calls out to the Rugged implementation' do + allow_any_instance_of(Rugged).to receive(:lookup).with(SeedRepo::Commit::ID) + + described_class.where(repository, SeedRepo::Commit::ID, 'files', false) end + + it_behaves_like :repo end end diff --git a/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb b/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb index c89913ec8e..bb10be2a4d 100644 --- a/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb +++ b/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb @@ -26,4 +26,14 @@ describe Gitlab::GitalyClient::StorageSettings do end end end + + describe '.disk_access_denied?' do + it 'return false when Rugged is enabled', :enable_rugged do + expect(described_class.disk_access_denied?).to be_falsey + end + + it 'returns true' do + expect(described_class.disk_access_denied?).to be_truthy + end + end end diff --git a/spec/lib/gitlab/middleware/basic_health_check_spec.rb b/spec/lib/gitlab/middleware/basic_health_check_spec.rb index 187d903a5e..86bdc479b6 100644 --- a/spec/lib/gitlab/middleware/basic_health_check_spec.rb +++ b/spec/lib/gitlab/middleware/basic_health_check_spec.rb @@ -28,6 +28,35 @@ describe Gitlab::Middleware::BasicHealthCheck do end end + context 'with X-Forwarded-For headers' do + let(:load_balancer_ip) { '1.2.3.4' } + + before do + env['HTTP_X_FORWARDED_FOR'] = "#{load_balancer_ip}, 127.0.0.1" + env['REMOTE_ADDR'] = '127.0.0.1' + env['PATH_INFO'] = described_class::HEALTH_PATH + end + + it 'returns 200 response when endpoint is allowed' do + allow(Settings.monitoring).to receive(:ip_whitelist).and_return([load_balancer_ip]) + expect(app).not_to receive(:call) + + response = middleware.call(env) + + expect(response[0]).to eq(200) + expect(response[1]).to eq({ 'Content-Type' => 'text/plain' }) + expect(response[2]).to eq(['GitLab OK']) + end + + it 'returns 404 when whitelist is not configured' do + allow(Settings.monitoring).to receive(:ip_whitelist).and_return([]) + + response = middleware.call(env) + + expect(response[0]).to eq(404) + end + end + context 'whitelisted IP' do before do env['REMOTE_ADDR'] = '127.0.0.1' diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb index fd443cc1f7..3ed57c2c91 100644 --- a/spec/lib/gitlab/request_context_spec.rb +++ b/spec/lib/gitlab/request_context_spec.rb @@ -6,6 +6,31 @@ describe Gitlab::RequestContext do let(:app) { -> (env) {} } let(:env) { Hash.new } + context 'with X-Forwarded-For headers', :request_store do + let(:load_balancer_ip) { '1.2.3.4' } + let(:headers) do + { + 'HTTP_X_FORWARDED_FOR' => "#{load_balancer_ip}, 127.0.0.1", + 'REMOTE_ADDR' => '127.0.0.1' + } + end + + let(:env) { Rack::MockRequest.env_for("/").merge(headers) } + + it 'returns the load balancer IP' do + client_ip = nil + + endpoint = proc do + client_ip = Gitlab::SafeRequestStore[:client_ip] + [200, {}, ["Hello"]] + end + + Rails.application.middleware.build(endpoint).call(env) + + expect(client_ip).to eq(load_balancer_ip) + end + end + context 'when RequestStore::Middleware is used' do around do |example| RequestStore::Middleware.new(-> (env) { example.run }).call({}) @@ -15,7 +40,7 @@ describe Gitlab::RequestContext do let(:ip) { '192.168.1.11' } before do - allow_any_instance_of(ActionDispatch::Request).to receive(:ip).and_return(ip) + allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip) described_class.new(app).call(env) end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index fd25132ed3..cc90a998d3 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -11,6 +11,25 @@ describe ApplicationRecord do end end + describe '.safe_ensure_unique' do + let(:model) { build(:suggestion) } + let(:klass) { model.class } + + before do + allow(model).to receive(:save).and_raise(ActiveRecord::RecordNotUnique) + end + + it 'returns false when ActiveRecord::RecordNotUnique is raised' do + expect(model).to receive(:save).once + expect(klass.safe_ensure_unique { model.save }).to be_falsey + end + + it 'retries based on retry count specified' do + expect(model).to receive(:save).exactly(3).times + expect(klass.safe_ensure_unique(retries: 2) { model.save }).to be_falsey + end + end + 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) diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index baad835218..9d4e18534a 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -542,7 +542,7 @@ eos end end - describe '#uri_type' do + shared_examples '#uri_type' do it 'returns the URI type at the given path' do expect(commit.uri_type('files/html')).to be(:tree) expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) @@ -561,6 +561,20 @@ eos end end + describe '#uri_type with Gitaly enabled' do + it_behaves_like "#uri_type" + end + + describe '#uri_type with Rugged enabled', :enable_rugged do + it 'calls out to the Rugged implementation' do + allow_any_instance_of(Rugged::Tree).to receive(:path).with('files/html').and_call_original + + commit.uri_type('files/html') + end + + it_behaves_like '#uri_type' + end + describe '.from_hash' do let(:new_commit) { described_class.from_hash(commit.to_hash, project) } diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index e530e0302f..53f5307ea0 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe MergeRequestDiff do + include RepoHelpers + let(:diff_with_commits) { create(:merge_request).merge_request_diff } describe 'validations' do @@ -194,6 +196,25 @@ describe MergeRequestDiff do expect(diff_file).to be_binary expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff) end + + context 'with diffs that contain a null byte' do + let(:filename) { 'test-null.txt' } + let(:content) { "a" * 10000 + "\x00" } + let(:project) { create(:project, :repository) } + let(:branch) { 'null-data' } + let(:target_branch) { 'master' } + + it 'saves diffs correctly' do + create_file_in_repo(project, target_branch, branch, filename, content) + + mr_diff = create(:merge_request, target_project: project, source_project: project, source_branch: branch, target_branch: target_branch).merge_request_diff + diff_file = mr_diff.merge_request_diff_files.find_by(new_path: filename) + + expect(diff_file).to be_binary + expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [filename]).to_a.first.diff) + expect(diff_file.diff).to include(content) + end + end end end diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index 13fe47799e..b26f656524 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -7,11 +7,43 @@ describe NotificationRecipient do subject(:recipient) { described_class.new(user, :watch, target: target, project: project) } - it 'denies access to a target when cross project access is denied' do - allow(Ability).to receive(:allowed?).and_call_original - expect(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(false) + describe '#has_access?' do + before do + allow(user).to receive(:can?).and_call_original + end - expect(recipient.has_access?).to be_falsy + context 'user cannot read cross project' do + it 'returns false' do + expect(user).to receive(:can?).with(:read_cross_project).and_return(false) + expect(recipient.has_access?).to eq false + end + end + + context 'user cannot read build' do + let(:target) { build(:ci_pipeline) } + + it 'returns false' do + expect(user).to receive(:can?).with(:read_build, target).and_return(false) + expect(recipient.has_access?).to eq false + end + end + + context 'user cannot read commit' do + let(:target) { build(:commit) } + + it 'returns false' do + expect(user).to receive(:can?).with(:read_commit, target).and_return(false) + expect(recipient.has_access?).to eq false + end + end + + context 'target has no policy' do + let(:target) { double.as_null_object } + + it 'returns true' do + expect(recipient.has_access?).to eq true + end + end end context '#notification_setting' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index f78760bf56..11926071a6 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2214,7 +2214,7 @@ describe Repository do rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id) end - describe '#ancestor?' do + shared_examples '#ancestor?' do let(:commit) { repository.commit } let(:ancestor) { commit.parents.first } @@ -2238,6 +2238,20 @@ describe Repository do end end + describe '#ancestor? with Gitaly enabled' do + it_behaves_like "#ancestor?" + end + + describe '#ancestor? with Rugged enabled', :enable_rugged do + it 'calls out to the Rugged implementation' do + allow_any_instance_of(Rugged).to receive(:merge_base).with(repository.commit.id, Gitlab::Git::BLANK_SHA).and_call_original + + repository.ancestor?(repository.commit.id, Gitlab::Git::BLANK_SHA) + end + + it_behaves_like '#ancestor?' + end + describe '#archive_metadata' do let(:ref) { 'master' } let(:storage_path) { '/tmp' } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 97e7a01922..e8d7b18bf0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -115,10 +115,17 @@ RSpec.configure do |config| TestEnv.clean_test_path end - config.before do + config.before do |example| # Enable all features by default for testing allow(Feature).to receive(:enabled?) { true } + enabled = example.metadata[:enable_rugged].present? + + # Disable Rugged features by default + Gitlab::Git::RuggedImpl::Repository::FEATURE_FLAGS.each do |flag| + allow(Feature).to receive(:enabled?).with(flag).and_return(enabled) + end + # The following can be removed when we remove the staged rollout strategy # and we can just enable it using instance wide settings # (ie. ApplicationSetting#auto_devops_enabled) diff --git a/spec/support/helpers/repo_helpers.rb b/spec/support/helpers/repo_helpers.rb index 3c6956cf5e..4af90f4af7 100644 --- a/spec/support/helpers/repo_helpers.rb +++ b/spec/support/helpers/repo_helpers.rb @@ -115,4 +115,18 @@ eos commits: commits ) end + + def create_file_in_repo( + project, start_branch, branch_name, filename, content, + commit_message: 'Add new content') + Files::CreateService.new( + project, + project.owner, + commit_message: commit_message, + start_branch: start_branch, + branch_name: branch_name, + file_path: filename, + file_content: content + ).execute + end end diff --git a/spec/validators/sha_validator_spec.rb b/spec/validators/sha_validator_spec.rb index b9242ef931..fa3dd68df2 100644 --- a/spec/validators/sha_validator_spec.rb +++ b/spec/validators/sha_validator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe ShaValidator do let(:validator) { described_class.new(attributes: [:base_commit_sha]) } - let(:merge_diff) { build(:merge_request_diff) } + let!(:merge_diff) { build(:merge_request_diff) } subject { validator.validate_each(merge_diff, :base_commit_sha, value) } @@ -10,6 +10,8 @@ describe ShaValidator do let(:value) { nil } it 'does not add any error if value is empty' do + expect(Commit).not_to receive(:valid_hash?) + subject expect(merge_diff.errors).to be_empty @@ -19,7 +21,9 @@ describe ShaValidator do context 'with valid sha' do let(:value) { Digest::SHA1.hexdigest(SecureRandom.hex) } - it 'does not add any error if value is empty' do + it 'does not add any error' do + expect(Commit).to receive(:valid_hash?).and_call_original + subject expect(merge_diff.errors).to be_empty @@ -30,6 +34,7 @@ describe ShaValidator do let(:value) { 'foo' } it 'adds error to the record' do + expect(Commit).to receive(:valid_hash?).and_call_original expect(merge_diff.errors).to be_empty subject