diff options
| -rw-r--r-- | .eslintrc.yml | 1 | ||||
| -rw-r--r-- | app/assets/javascripts/filtered_search/.eslintrc.yml | 3 | ||||
| -rw-r--r-- | app/assets/javascripts/ide/.eslintrc.yml | 2 | ||||
| -rw-r--r-- | app/assets/javascripts/image_diff/.eslintrc.yml | 3 | ||||
| -rw-r--r-- | app/finders/user_finder.rb | 6 | ||||
| -rw-r--r-- | doc/administration/high_availability/README.md | 31 | ||||
| -rw-r--r-- | doc/development/documentation/styleguide.md | 13 | ||||
| -rw-r--r-- | lib/api/helpers/internal_helpers.rb | 4 | ||||
| -rw-r--r-- | lib/api/internal/base.rb | 63 | ||||
| -rw-r--r-- | lib/api/support/git_access_actor.rb | 12 | ||||
| -rw-r--r-- | spec/finders/user_finder_spec.rb | 22 | ||||
| -rw-r--r-- | spec/lib/api/support/git_access_actor_spec.rb | 15 | ||||
| -rw-r--r-- | spec/requests/api/internal/base_spec.rb | 11 |
13 files changed, 99 insertions, 87 deletions
diff --git a/.eslintrc.yml b/.eslintrc.yml index 6366feaf897..f95c8cf58af 100644 --- a/.eslintrc.yml +++ b/.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 diff --git a/app/assets/javascripts/filtered_search/.eslintrc.yml b/app/assets/javascripts/filtered_search/.eslintrc.yml new file mode 100644 index 00000000000..9faca7152f6 --- /dev/null +++ b/app/assets/javascripts/filtered_search/.eslintrc.yml @@ -0,0 +1,3 @@ +rules: + # https://gitlab.com/gitlab-org/gitlab/issues/28716 + import/no-cycle: off diff --git a/app/assets/javascripts/ide/.eslintrc.yml b/app/assets/javascripts/ide/.eslintrc.yml index 92b96d717be..2af5c1798f5 100644 --- a/app/assets/javascripts/ide/.eslintrc.yml +++ b/app/assets/javascripts/ide/.eslintrc.yml @@ -1,3 +1,5 @@ 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 diff --git a/app/assets/javascripts/image_diff/.eslintrc.yml b/app/assets/javascripts/image_diff/.eslintrc.yml new file mode 100644 index 00000000000..bf9e184381e --- /dev/null +++ b/app/assets/javascripts/image_diff/.eslintrc.yml @@ -0,0 +1,3 @@ +rules: + # https://gitlab.com/gitlab-org/gitlab/issues/28719 + import/no-cycle: off diff --git a/app/finders/user_finder.rb b/app/finders/user_finder.rb index 1dd1a27437e..556be4c4338 100644 --- a/app/finders/user_finder.rb +++ b/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 diff --git a/doc/administration/high_availability/README.md b/doc/administration/high_availability/README.md index f3a8475d75d..dad83b633a2 100644 --- a/doc/administration/high_availability/README.md +++ b/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 diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index fac0a581957..b604c94e016 100644 --- a/doc/development/documentation/styleguide.md +++ b/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 diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index dfac777e4a1..b03eb5ad440 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/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 diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index c70f2f3e2c8..50142b8641e 100644 --- a/lib/api/internal/base.rb +++ b/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]) + actor.update_last_used_at! + user = actor.user - if key - key.update_last_used_at - else - break { 'success' => false, 'message' => 'Could not find the given key' } + 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 diff --git a/lib/api/support/git_access_actor.rb b/lib/api/support/git_access_actor.rb index 2e0962c6295..cb9bf4472eb 100644 --- a/lib/api/support/git_access_actor.rb +++ b/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 diff --git a/spec/finders/user_finder_spec.rb b/spec/finders/user_finder_spec.rb index c20d7850d68..b89b422aa2c 100644 --- a/spec/finders/user_finder_spec.rb +++ b/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 diff --git a/spec/lib/api/support/git_access_actor_spec.rb b/spec/lib/api/support/git_access_actor_spec.rb index 63f5966faea..69637947c79 100644 --- a/spec/lib/api/support/git_access_actor_spec.rb +++ b/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 diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index fd3d1ebee20..7292e7a6a4e 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/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 |