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

6 files changed, 41 insertions, 21 deletions

diff --git a/CHANGELOG b/CHANGELOG
index d4268fc3709..392a356c79a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -48,6 +48,7 @@ v 8.13.0 (unreleased)
   - Prevent flash alert text from being obscured when container is fluid
   - Append issue template to existing description !6149 (Joseph Frazier)
   - Trending projects now only show public projects and the list of projects is cached for a day
+  - Memoize Gitlab Shell's secret token (!6599, Justin DiPierro)
   - Revoke button in Applications Settings underlines on hover.
   - Use higher size on Gitlab::Redis connection pool on Sidekiq servers
   - Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska)
diff --git a/config/initializers/gitlab_shell_secret_token.rb b/config/initializers/gitlab_shell_secret_token.rb
index 7454c33c9dd..529dcdd4644 100644
--- a/config/initializers/gitlab_shell_secret_token.rb
+++ b/config/initializers/gitlab_shell_secret_token.rb
@@ -1 +1 @@
-Gitlab::Shell.new.generate_and_link_secret_token
+Gitlab::Shell.ensure_secret_token!
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"
diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb
index 07407f212aa..f826d0d1b04 100644
--- a/spec/lib/gitlab/backend/shell_spec.rb
+++ b/spec/lib/gitlab/backend/shell_spec.rb
@@ -22,15 +22,15 @@ describe Gitlab::Shell, lib: true do
 
   it { expect(gitlab_shell.url_to_repo('diaspora')).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git") }
 
-  describe 'generate_and_link_secret_token' do
+  describe 'memoized secret_token' do
     let(:secret_file) { 'tmp/tests/.secret_shell_test' }
     let(:link_file) { 'tmp/tests/shell-secret-test/.gitlab_shell_secret' }
 
     before do
-      allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test')
       allow(Gitlab.config.gitlab_shell).to receive(:secret_file).and_return(secret_file)
+      allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test')
       FileUtils.mkdir('tmp/tests/shell-secret-test')
-      gitlab_shell.generate_and_link_secret_token
+      Gitlab::Shell.ensure_secret_token!
     end
 
     after do
@@ -39,7 +39,10 @@ describe Gitlab::Shell, lib: true do
     end
 
     it 'creates and links the secret token file' do
+      secret_token = Gitlab::Shell.secret_token
+
       expect(File.exist?(secret_file)).to be(true)
+      expect(File.read(secret_file).chomp).to eq(secret_token)
       expect(File.symlink?(link_file)).to be(true)
       expect(File.readlink(link_file)).to eq(secret_file)
     end