New upstream version 11.8.9

.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

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)

GITALY_SERVER_VERSION

@@ -1 +1 @@
-1.20.0
+1.20.1

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

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)

VERSION

@@ -1 +1 @@
-11.8.6
+11.8.9

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,
+    };
   }
 }

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

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

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

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

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

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
 

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

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

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

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'

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

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.

ee/changelogs/unreleased/security-milestone-labels.yml

@@ -1,5 +0,0 @@
----
-title: Check label_ids parent when updating issue board
-merge_request:
-author:
-type: security

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

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

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

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

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

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

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

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

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

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

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')

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)
 

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
 

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

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

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) }

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

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

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) }

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' }

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

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

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'

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
 

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)

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) }
 

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
 

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

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' }

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)

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

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