Add authorized_keys_enabled to Application Settings

app/controllers/admin/application_settings_controller.rb

@@ -177,7 +177,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :check_namespace_plan,
       :mirror_max_delay,
       :mirror_max_capacity,
-      :mirror_capacity_threshold
+      :mirror_capacity_threshold,
+      :authorized_keys_enabled
     ]
   end
 end

app/models/ee/application_setting.rb

@@ -32,6 +32,7 @@ module EE
     module ClassMethods
       def defaults
         super.merge(
+          authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
           elasticsearch_url: ENV['ELASTIC_URL'] || 'http://localhost:9200',
           elasticsearch_aws: false,
           elasticsearch_aws_region: ENV['ELASTIC_REGION'] || 'us-east-1',

app/views/admin/application_settings/_form.html.haml

@@ -637,6 +637,20 @@
           installations. Set to 0 to completely disable polling.
           = link_to icon('question-circle'), help_page_path('administration/polling')
 
+  %fieldset
+    %legend Performance optimization
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :authorized_keys_enabled do
+            = f.check_box :authorized_keys_enabled
+            Write to "authorized_keys" file
+          .help-block
+            By default, we write to the "authorized_keys" file to support old
+            OpenSSH servers. If, and only if, you have configured the GitLab
+            instance to use the AuthorizedKeysCommand, uncheck this to improve
+            performance.
+
   - if Gitlab::Geo.license_allows?
     %fieldset
       %legend GitLab Geo

db/migrate/20170531180233_add_authorized_keys_enabled_to_application_settings.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddAuthorizedKeysEnabledToApplicationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    # allow_null: true because we want to set the default based on if the
+    # instance is configured to use AuthorizedKeysCommand
+    add_column :application_settings, :authorized_keys_enabled, :boolean, allow_null: true
+  end
+end

db/schema.rb

@@ -140,6 +140,7 @@ ActiveRecord::Schema.define(version: 20170602003304) do
     t.integer "mirror_max_delay", default: 5, null: false
     t.integer "mirror_max_capacity", default: 100, null: false
     t.integer "mirror_capacity_threshold", default: 50, null: false
+    t.boolean "authorized_keys_enabled"
   end
 
   create_table "approvals", force: :cascade do |t|

lib/gitlab/shell.rb

@@ -197,6 +197,8 @@ module Gitlab
     #   add_key("key-42", "sha-rsa ...")
     #
     def add_key(key_id, key_content)
+      return unless self.authorized_keys_enabled?
+
       Gitlab::Utils.system_silent([gitlab_shell_keys_path,
                                    'add-key', key_id, self.class.strip_key(key_content)])
     end
@@ -206,6 +208,8 @@ module Gitlab
     # Ex.
     #   batch_add_keys { |adder| adder.add_key("key-42", "sha-rsa ...") }
     def batch_add_keys(&block)
+      return unless self.authorized_keys_enabled?
+
       IO.popen(%W(#{gitlab_shell_path}/bin/gitlab-keys batch-add-keys), 'w') do |io|
         yield(KeyAdder.new(io))
       end
@@ -217,6 +221,8 @@ module Gitlab
     #   remove_key("key-342", "sha-rsa ...")
     #
     def remove_key(key_id, key_content)
+      return unless self.authorized_keys_enabled?
+
       Gitlab::Utils.system_silent([gitlab_shell_keys_path,
                                    'rm-key', key_id, key_content])
     end
@@ -227,6 +233,8 @@ module Gitlab
     #   remove_all_keys
     #
     def remove_all_keys
+      return unless self.authorized_keys_enabled?
+
       Gitlab::Utils.system_silent([gitlab_shell_keys_path, 'clear'])
     end
 
@@ -356,5 +364,9 @@ module Gitlab
     def gitlab_shell_keys_path
       File.join(gitlab_shell_path, 'bin', 'gitlab-keys')
     end
+
+    def authorized_keys_enabled?
+      current_application_settings.authorized_keys_enabled
+    end
   end
 end

spec/lib/gitlab/shell_spec.rb

@@ -104,13 +104,101 @@ describe Gitlab::Shell, lib: true do
   end
 
   describe '#add_key' do
-    it 'removes trailing garbage' do
-      allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
-      expect(Gitlab::Utils).to receive(:system_silent).with(
-        [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar']
-      )
+    context 'when authorized_keys_enabled is true' do
+      it 'removes trailing garbage' do
+        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
+        expect(Gitlab::Utils).to receive(:system_silent).with(
+          [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar']
+        )
+
+        gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+      end
+    end
+
+    context 'when authorized_keys_enabled is false' do
+      before do
+        stub_application_setting(authorized_keys_enabled: false)
+      end
+
+      it 'does nothing' do
+        expect(Gitlab::Utils).not_to receive(:system_silent)
+
+        gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+      end
+    end
+  end
+
+  describe '#batch_add_keys' do
+    context 'when authorized_keys_enabled is true' do
+      it 'instantiates KeyAdder' do
+        expect_any_instance_of(Gitlab::Shell::KeyAdder).to receive(:add_key).with('key-123', 'ssh-rsa foobar')
+
+        gitlab_shell.batch_add_keys do |adder|
+          adder.add_key('key-123', 'ssh-rsa foobar')
+        end
+      end
+    end
+
+    context 'when authorized_keys_enabled is false' do
+      before do
+        stub_application_setting(authorized_keys_enabled: false)
+      end
+
+      it 'does nothing' do
+        expect_any_instance_of(Gitlab::Shell::KeyAdder).not_to receive(:add_key)
+
+        gitlab_shell.batch_add_keys do |adder|
+          adder.add_key('key-123', 'ssh-rsa foobar')
+        end
+      end
+    end
+  end
 
-      gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+  describe '#remove_key' do
+    context 'when authorized_keys_enabled is true' do
+      it 'removes trailing garbage' do
+        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
+        expect(Gitlab::Utils).to receive(:system_silent).with(
+          [:gitlab_shell_keys_path, 'rm-key', 'key-123', 'ssh-rsa foobar']
+        )
+
+        gitlab_shell.remove_key('key-123', 'ssh-rsa foobar')
+      end
+    end
+
+    context 'when authorized_keys_enabled is false' do
+      before do
+        stub_application_setting(authorized_keys_enabled: false)
+      end
+
+      it 'does nothing' do
+        expect(Gitlab::Utils).not_to receive(:system_silent)
+
+        gitlab_shell.remove_key('key-123', 'ssh-rsa foobar')
+      end
+    end
+  end
+
+  describe '#remove_all_keys' do
+    context 'when authorized_keys_enabled is true' do
+      it 'removes trailing garbage' do
+        allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
+        expect(Gitlab::Utils).to receive(:system_silent).with([:gitlab_shell_keys_path, 'clear'])
+
+        gitlab_shell.remove_all_keys
+      end
+    end
+
+    context 'when authorized_keys_enabled is false' do
+      before do
+        stub_application_setting(authorized_keys_enabled: false)
+      end
+
+      it 'does nothing' do
+        expect(Gitlab::Utils).not_to receive(:system_silent)
+
+        gitlab_shell.remove_all_keys
+      end
     end
   end