diff options
author | Rémy Coutable <remy@rymai.me> | 2016-10-07 10:35:03 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-10-07 10:35:03 +0000 |
commit | 0876b46024be019d9ef9846045d3941dcdf90981 (patch) | |
tree | 84ea7e0cd4a377963a4e4af28f683bb1fa323912 /lib | |
parent | 1be151621721d9e89c4c6514b8523e999e5e0fed (diff) | |
parent | 1c462cf7d6d61aebac9b909102f0e794cc9e409a (diff) | |
download | gitlab-ce-0876b46024be019d9ef9846045d3941dcdf90981.tar.gz |
Merge branch 'memoize_shell_secret_token' into 'master'
Memoize Github::Shell's secret token
## What does this MR do?
`API::Helpers#secret_token` was reading the secret file on every invocation. This MR reads the file in the `gitlab_shell_secret_token.rb` initializer and saves it as a class variable at `Gitlab::Shell.secret_token`
## Are there points in the code the reviewer needs to double check?
- I'm not sure if the use of `cattr_accessor` is the best approach, or if should be moved into the `class << self` block?
- Should `API::Helpers#secret_token` be removed in favor of using `Gitlab::Shell.secret_token`?
## Why was this MR needed?
Performance optimization.
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/22510
See merge request !6599
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/helpers.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/backend/shell.rb | 46 | ||||
-rw-r--r-- | lib/tasks/gitlab/shell.rake | 2 |
3 files changed, 33 insertions, 17 deletions
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 281a8f13531..67473f300c9 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -433,7 +433,7 @@ module API end def secret_token - File.read(Gitlab.config.gitlab_shell.secret_file).chomp + Gitlab::Shell.secret_token end def send_git_blob(repository, blob) diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb index 79eac66b364..d0060fbaca1 100644 --- a/lib/gitlab/backend/shell.rb +++ b/lib/gitlab/backend/shell.rb @@ -17,6 +17,18 @@ module Gitlab end class << self + def secret_token + @secret_token ||= begin + File.read(Gitlab.config.gitlab_shell.secret_file).chomp + end + end + + def ensure_secret_token! + return if File.exist?(File.join(Gitlab.config.gitlab_shell.path, '.gitlab_shell_secret')) + + generate_and_link_secret_token + end + def version_required @version_required ||= File.read(Rails.root. join('GITLAB_SHELL_VERSION')).strip @@ -25,6 +37,25 @@ module Gitlab def strip_key(key) key.split(/ /)[0, 2].join(' ') end + + private + + # Create (if necessary) and link the secret token file + def generate_and_link_secret_token + secret_file = Gitlab.config.gitlab_shell.secret_file + shell_path = Gitlab.config.gitlab_shell.path + + unless File.size?(secret_file) + # Generate a new token of 16 random hexadecimal characters and store it in secret_file. + token = SecureRandom.hex(16) + File.write(secret_file, token) + end + + link_path = File.join(shell_path, '.gitlab_shell_secret') + if File.exist?(shell_path) && !File.exist?(link_path) + FileUtils.symlink(secret_file, link_path) + end + end end # Init new repository @@ -201,21 +232,6 @@ module Gitlab File.exist?(full_path(storage, dir_name)) end - # Create (if necessary) and link the secret token file - def generate_and_link_secret_token - secret_file = Gitlab.config.gitlab_shell.secret_file - unless File.size?(secret_file) - # Generate a new token of 16 random hexadecimal characters and store it in secret_file. - token = SecureRandom.hex(16) - File.write(secret_file, token) - end - - link_path = File.join(gitlab_shell_path, '.gitlab_shell_secret') - if File.exist?(gitlab_shell_path) && !File.exist?(link_path) - FileUtils.symlink(secret_file, link_path) - end - end - protected def gitlab_shell_path diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index bb7eb852f1b..210899882b4 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -78,7 +78,7 @@ namespace :gitlab do f.puts "PATH=#{ENV['PATH']}" end - Gitlab::Shell.new.generate_and_link_secret_token + Gitlab::Shell.ensure_secret_token! end desc "GitLab | Setup gitlab-shell" |