Add latest changes from gitlab-org/gitlab@master

.eslintrc.yml

@@ -30,7 +30,6 @@ rules:
   no-else-return:
     - error
     - allowElseIf: true
-  import/no-cycle: warn
   import/no-useless-path-segments: off
   import/order: warn
   lines-between-class-members: off

app/assets/javascripts/filtered_search/.eslintrc.yml

+rules:
+  # https://gitlab.com/gitlab-org/gitlab/issues/28716
+  import/no-cycle: off

app/assets/javascripts/ide/.eslintrc.yml

 rules:
+  # https://gitlab.com/gitlab-org/gitlab/issues/28717
+  import/no-cycle: off
   # https://gitlab.com/gitlab-org/gitlab/issues/33024
   promise/no-nesting: off

app/assets/javascripts/image_diff/.eslintrc.yml

+rules:
+  # https://gitlab.com/gitlab-org/gitlab/issues/28719
+  import/no-cycle: off

app/finders/user_finder.rb

@@ -52,12 +52,6 @@ class UserFinder
     end
   end
-  def find_by_ssh_key_id
-    return unless input_is_id?
-
-    User.find_by_ssh_key_id(@username_or_id)
-  end
-
   def input_is_id?
     @username_or_id.is_a?(Numeric) || @username_or_id =~ /^\d+$/
   end

doc/administration/high_availability/README.md

@@ -219,6 +219,37 @@ Note that your exact needs may be more, depending on your workload. Your
 workload is influenced by factors such as - but not limited to - how active your
 users are, how much automation you use, mirroring, and repo/change size.
+### 2,000 User Configuration
+
+- **Supported Users (approximate):** 2,000
+- **Test RPS Rates:** API: 40 RPS, Web: 4 RPS, Git: 4 RPS
+- **Status:** Work-in-progress
+- **Known Issues:** For the latest list of known performance issues head
+[here](https://gitlab.com/gitlab-org/gitlab/issues?label_name%5B%5D=Quality%3Aperformance-issues).
+
+NOTE: **Note:**  This architecture is a work-in-progress of the work so far. The
+Quality team will be certifying this environment in late 2019 or early 2020. The specifications
+may be adjusted prior to certification based on performance testing.
+
+| Service                     | Nodes | Configuration         | GCP type      |
+| ----------------------------|-------|-----------------------|---------------|
+| GitLab Rails <br> - Puma workers on each node set to 90% of available CPUs with 8 threads | 3 | 8 vCPU, 7.2GB Memory | n1-highcpu-8 |
+| PostgreSQL                  | 3     | 2 vCPU, 7.5GB Memory  | n1-standard-2 |
+| PgBouncer                   | 3     | 2 vCPU, 1.8GB Memory  | n1-highcpu-2  |
+| Gitaly <br> - Gitaly Ruby workers on each node set to 20% of available CPUs | X[^1] . | 4 vCPU, 15GB Memory   | n1-standard-4 |
+| Redis Cache + Sentinel <br> - Cache maxmemory set to 90% of available memory | 3 | 2 vCPU, 7.5GB Memory | n1-standard-2 |
+| Redis Persistent + Sentinel | 3     | 2 vCPU, 7.5GB Memory  | n1-standard-2 |
+| Sidekiq                     | 4     | 2 vCPU, 7.5GB Memory  | n1-standard-2 |
+| Consul                      | 3     | 2 vCPU, 1.8GB Memory  | n1-highcpu-2  |
+| NFS Server[^4] .            | 1     | 4 vCPU, 3.6GB Memory  | n1-highcpu-4  |
+| S3 Object Storage[^3] .     | -     | -                     | -             |
+| Monitoring node             | 1     | 2 vCPU, 1.8GB Memory  | n1-highcpu-2  |
+| External load balancing node[^2] . | 1 | 2 vCPU, 1.8GB Memory | n1-highcpu-2 |
+| Internal load balancing node[^2] . | 1 | 2 vCPU, 1.8GB Memory | n1-highcpu-2 |
+
+NOTE: **Note:** Memory values are given directly by GCP machine sizes. On different cloud
+vendors a best effort like for like can be used.
+
 ### 5,000 User Configuration
 - **Supported Users (approximate):** 5,000

doc/development/documentation/styleguide.md

@@ -212,7 +212,7 @@ Do not include the same information in multiple places. [Link to a SSOT instead.
 - Use inclusive language and avoid jargon, as well as uncommon
   words. The docs should be clear and easy to understand.
-- Write in the 3rd person (use "we," "you," "us," "one," instead of "I" or "me").
+- Do not write in the first person singular. Instead of "I" or "me," use "we," "you," "us," or "one."
 - Be clear, concise, and stick to the goal of the doc.
 - Write in US English with US grammar.
 - Capitalize "G" and "L" in GitLab.
@@ -230,18 +230,23 @@ Do not include the same information in multiple places. [Link to a SSOT instead.
     "Create a new merge request for Z."
 - Avoid use of the future tense:
-  - Instead of, "After you execute this command, the result will be displayed," say "After you execute this command, the result is displayed."
+  - Instead of "after you execute this command, GitLab will display the result", use "after you execute this command, GitLab displays the result".
   - Only use the future tense to convey when the action or result will actually occur at a future time.
 - Do not use contractions:
-  - Instead of "don't," "can't," "doesn't," and so on, say "do not," "cannot," or "does not."
+  - Instead of "don't," "can't," "doesn't," and so on, use "do not," "cannot," or "does not."
   - Possible exceptions are cases when a more familiar tone is desired, such as a blog post or other casual context.
 - Do not use slashes to clump different words together or as a replacement for the word "or":
-  - Instead of "and/or," consider saying "or," or use another sensible construction.
+  - Instead of "and/or," consider using "or," or use another sensible construction.
   - Other examples include "clone/fetch," author/assignee," and "namespace/repository name." Break apart any such instances in an appropriate way.
   - Exceptions to this rule include commonly accepted technical terms such as CI/CD, TCP/IP, and so on.
 - Do not use "may" and "might" interchangeably:
   - Use "might" to indicate the probability of something occurring. "If you skip this step, the import process might fail."
   - Use "may" to indicate giving permission for someone to do something, or consider using "can" instead. "You may select either option on this screen." Or, "you can select either option on this screen."
+- We recommend avoiding Latin abbreviations, such as "e.g.," "i.e.," or "etc.,"
+as even native users of English might misunderstand them.
+  - Instead of "i.e.", use "that is."
+  - Instead of "e.g.", use "for example."
+  - Instead of "etc.", either use "and so on" or consider editing it out, since it can be vague.
 ## Text

lib/api/helpers/internal_helpers.rb

@@ -7,6 +7,10 @@ module API
       delegate :wiki?, to: :repo_type
+      def actor
+        @actor ||= Support::GitAccessActor.from_params(params)
+      end
+
       def repo_type
         set_project unless defined?(@repo_type) # rubocop:disable Gitlab/ModuleWithInstanceVariables
         @repo_type # rubocop:disable Gitlab/ModuleWithInstanceVariables

lib/api/internal/base.rb

@@ -7,7 +7,6 @@ module API
       before { authenticate_by_gitlab_shell_token! }
       helpers ::API::Helpers::InternalHelpers
-      helpers ::Gitlab::Identifier
       UNKNOWN_CHECK_RESULT_ERROR = 'Unknown check result'.freeze
@@ -35,7 +34,6 @@ module API
           env = parse_env
           Gitlab::Git::HookEnv.set(gl_repository, env) if project
-          actor = Support::GitAccessActor.from_params(params)
           actor.update_last_used_at!
           access_checker = access_checker_for(actor, params[:protocol])
@@ -103,36 +101,30 @@ module API
           check_allowed(params)
         end
-        # rubocop: disable CodeReuse/ActiveRecord
         post "/lfs_authenticate" do
           status 200
-          if params[:key_id]
-            actor = Key.find(params[:key_id])
-            actor.update_last_used_at
-          elsif params[:user_id]
-            actor = User.find_by(id: params[:user_id])
-            raise ActiveRecord::RecordNotFound.new("No such user id!") unless actor
-          else
-            raise ActiveRecord::RecordNotFound.new("No key_id or user_id passed!")
+          unless actor.key_or_user
+            raise ActiveRecord::RecordNotFound.new('User not found!')
           end
+          actor.update_last_used_at!
+
           Gitlab::LfsToken
-            .new(actor)
+            .new(actor.key_or_user)
             .authentication_payload(lfs_authentication_url(project))
         end
-        # rubocop: enable CodeReuse/ActiveRecord
         #
         # Get a ssh key using the fingerprint
         #
         # rubocop: disable CodeReuse/ActiveRecord
-        get "/authorized_keys" do
+        get '/authorized_keys' do
           fingerprint = params.fetch(:fingerprint) do
             Gitlab::InsecureKeyFingerprint.new(params.fetch(:key)).fingerprint
           end
           key = Key.find_by(fingerprint: fingerprint)
-          not_found!("Key") if key.nil?
+          not_found!('Key') if key.nil?
           present key, with: Entities::SSHKey
         end
         # rubocop: enable CodeReuse/ActiveRecord
@@ -141,16 +133,10 @@ module API
         # Discover user by ssh key, user id or username
         #
         get '/discover' do
-          if params[:key_id]
-            user = UserFinder.new(params[:key_id]).find_by_ssh_key_id
-          elsif params[:username]
-            user = UserFinder.new(params[:username]).find_by_username
-          end
-
-          present user, with: Entities::UserSafe
+          present actor.user, with: Entities::UserSafe
         end
-        get "/check" do
+        get '/check' do
           {
             api_version: API.version,
             gitlab_version: Gitlab::VERSION,
@@ -158,35 +144,26 @@ module API
             redis: redis_ping
           }
         end
-
-        # rubocop: disable CodeReuse/ActiveRecord
         post '/two_factor_recovery_codes' do
           status 200
-          if params[:key_id]
-            key = Key.find_by(id: params[:key_id])
-            if key
-              key.update_last_used_at
-            else
-              break { 'success' => false, 'message' => 'Could not find the given key' }
+          actor.update_last_used_at!
+          user = actor.user
+          if params[:key_id]
+            unless actor.key
+              break { success: false, message: 'Could not find the given key' }
             end
-            if key.is_a?(DeployKey)
+            if actor.key.is_a?(DeployKey)
               break { success: false, message: 'Deploy keys cannot be used to retrieve recovery codes' }
             end
-            user = key.user
-
             unless user
               break { success: false, message: 'Could not find a user for the given key' }
             end
-          elsif params[:user_id]
-            user = User.find_by(id: params[:user_id])
-
-            unless user
-              break { success: false, message: 'Could not find the given user' }
-            end
+          elsif params[:user_id] && user.nil?
+            break { success: false, message: 'Could not find the given user' }
           end
           unless user.two_factor_enabled?
@@ -201,7 +178,6 @@ module API
           { success: true, recovery_codes: codes }
         end
-        # rubocop: enable CodeReuse/ActiveRecord
         post '/pre_receive' do
           status 200
@@ -211,7 +187,7 @@ module API
           { reference_counter_increased: reference_counter_increased }
         end
-        post "/notify_post_receive" do
+        post '/notify_post_receive' do
           status 200
           # TODO: Re-enable when Gitaly is processing the post-receive notification
@@ -229,8 +205,7 @@ module API
           status 200
           response = Gitlab::InternalPostReceive::Response.new
-          user = identify(params[:identifier])
-          project = Gitlab::GlRepository.parse(params[:gl_repository]).first
+          user = actor.user
           push_options = Gitlab::PushOptions.new(params[:push_options])
           response.reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease

lib/api/support/git_access_actor.rb

@@ -3,7 +3,9 @@
 module API
   module Support
     class GitAccessActor
-      attr_reader :user
+      extend ::Gitlab::Identifier
+
+      attr_reader :user, :key
       def initialize(user: nil, key: nil)
         @user = user
@@ -19,6 +21,10 @@ module API
           new(user: UserFinder.new(params[:user_id]).find_by_id)
         elsif params[:username]
           new(user: UserFinder.new(params[:username]).find_by_username)
+        elsif params[:identifier]
+          new(user: identify(params[:identifier]))
+        else
+          new
         end
       end
@@ -33,10 +39,6 @@ module API
       def update_last_used_at!
         key&.update_last_used_at
       end
-
-      private
-
-      attr_reader :key
     end
   end
 end

spec/finders/user_finder_spec.rb

@@ -176,26 +176,4 @@ describe UserFinder do
       end
     end
   end
-
-  describe '#find_by_ssh_key_id' do
-    let_it_be(:ssh_key) { create(:key, user: user) }
-
-    it 'returns the user when passing the ssh key id' do
-      found = described_class.new(ssh_key.id).find_by_ssh_key_id
-
-      expect(found).to eq(user)
-    end
-
-    it 'returns the user when passing the ssh key id (string)' do
-      found = described_class.new(ssh_key.id.to_s).find_by_ssh_key_id
-
-      expect(found).to eq(user)
-    end
-
-    it 'returns nil when the id does not exist' do
-      found = described_class.new(-1).find_by_ssh_key_id
-
-      expect(found).to be_nil
-    end
-  end
 end

spec/lib/api/support/git_access_actor_spec.rb

@@ -9,17 +9,26 @@ describe API::Support::GitAccessActor do
   subject { described_class.new(user: user, key: key) }
   describe '.from_params' do
+    let(:key) { create(:key) }
+
     context 'with params that are valid' do
       it 'returns an instance of API::Support::GitAccessActor' do
-        params = { key_id: create(:key).id }
+        params = { key_id: key.id }
         expect(described_class.from_params(params)).to be_instance_of(described_class)
       end
     end
     context 'with params that are invalid' do
-      it 'returns nil' do
-        expect(described_class.from_params({})).to be_nil
+      it "returns an instance of #{described_class}" do
+        expect(described_class.from_params({})).to be_instance_of(described_class)
+      end
+    end
+
+    context 'when passing an identifier used gitaly' do
+      it 'finds the user based on an identifier' do
+        expect(described_class).to receive(:identify).and_call_original
+        expect(described_class.from_params(identifier: "key-#{key.id}").user).to eq(key.user)
       end
     end
   end

spec/requests/api/internal/base_spec.rb

@@ -193,7 +193,15 @@ describe API::Internal::Base do
     end
     it 'responds successfully when a user is not found' do
-      get(api("/internal/discover"), params: { username: 'noone', secret_token: secret_token })
+      get(api('/internal/discover'), params: { username: 'noone', secret_token: secret_token })
+
+      expect(response).to have_gitlab_http_status(200)
+
+      expect(response.body).to eq('null')
+    end
+
+    it 'response successfully when passing invalid params' do
+      get(api('/internal/discover'), params: { nothing: 'to find a user', secret_token: secret_token })
       expect(response).to have_gitlab_http_status(200)
@@ -819,7 +827,6 @@ describe API::Internal::Base do
     before do
       project.add_developer(user)
-      allow(described_class).to receive(:identify).and_return(user)
       allow_any_instance_of(Gitlab::Identifier).to receive(:identify).and_return(user)
     end