Merge branch '3340-geo-secondaries-can-have-ssh-config-for-now' into 'master'

Stop needing SSH keys for Geo primaries

Closes #3340

See merge request !2861

app/assets/javascripts/geo/geo_node_form.js

 export default class GeoNodeForm {
   constructor(container) {
     this.$container = container;
-    this.$namespaces = this.$container.find('.js-namespaces');
+    this.$namespaces = this.$container.find('.js-hide-if-geo-primary');
     this.$namespacesSelect = this.$namespaces.find('.select2');
     this.$primaryCheckbox = this.$container.find("input[type='checkbox']");
     this.$primaryCheckbox.on('change', () => this.onPrimaryCheckboxChange());

app/models/geo_node.rb

@@ -4,7 +4,7 @@ class GeoNode < ActiveRecord::Base
   ignore_column :system_hook_id
-  belongs_to :geo_node_key, dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent
+  belongs_to :geo_node_key, inverse_of: :geo_node, dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent
   belongs_to :oauth_application, class_name: 'Doorkeeper::Application', dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent
   has_many :geo_node_namespace_links
@@ -25,6 +25,8 @@ class GeoNode < ActiveRecord::Base
   validates :access_key, presence: true
   validates :encrypted_secret_access_key, presence: true
+  validates :geo_node_key, presence: true, if: :secondary?
+
   after_initialize :build_dependents
   after_save :expire_cache!
   after_destroy :expire_cache!
@@ -196,13 +198,15 @@ def url_helper_args
   end
   def build_dependents
-    unless persisted?
-      self.build_geo_node_key unless geo_node_key.present?
-    end
+    build_geo_node_key if new_record? && secondary? && geo_node_key.nil?
   end
   def update_dependents_attributes
-    self.geo_node_key&.title = "Geo node: #{self.url}"
+    if primary?
+      self.geo_node_key = nil
+    else
+      self.geo_node_key&.title = "Geo node: #{self.url}"
+    end
     if self.primary?
       self.oauth_application = nil

app/models/geo_node_key.rb

 class GeoNodeKey < Key
-  has_one :geo_node
+  has_one :geo_node, inverse_of: :geo_node_key
   def orphaned?
     self.geo_nodes.length == 0

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

@@ -12,8 +12,9 @@
   .col-sm-10
     = form.text_field :url, class: 'form-control'
+
 = form.fields_for :geo_node_key, geo_node.geo_node_key, include_id: !disable_key_edit do |fg|
-  .form-group
+  .form-group.js-hide-if-geo-primary{ class: ('hidden' unless geo_node.secondary?) }
     = fg.label :key, 'Public Key', class: 'control-label'
     .col-sm-10
       = fg.text_area :key, class: 'form-control thin_area', rows: 5, disabled: disable_key_edit
@@ -22,7 +23,7 @@
           Paste the ssh public key used by the node you are adding. Read more about it
           = link_to 'here', help_page_path('gitlab-geo/configuration.html', anchor: 'step-5-enabling-the-secondary-gitlab-node')
-.form-group.js-namespaces{ class: ('hidden' unless geo_node.new_record? || geo_node.secondary?) }
+.form-group.js-hide-if-geo-primary{ class: ('hidden' unless geo_node.secondary?) }
   = form.label :namespace_ids, 'Namespaces to replicate', class: 'control-label'
   .col-sm-10
     = form.select :namespace_ids, namespaces_options(geo_node.namespace_ids), { include_hidden: true }, multiple: true, class: 'select2 select-wide', data: { field: 'namespace_ids' }

changelogs/unreleased-ee/3340-geo-secondaries-can-have-ssh-config-for-now.yml

+---
+title: Geo primary nodes no longer require SSH keys
+merge_request: 2861
+author:
+type: changed

doc/gitlab-geo/configuration.md

@@ -62,15 +62,13 @@ logins opened on all nodes as we will be moving back and forth.
     sudo -i
     ```
-1. Added in GitLab 9.1: Execute the command below to define the node as primary Geo node:
+1. Execute the command below to define the node as primary Geo node:
     ```
     gitlab-ctl set-geo-primary-node
     ```
-    This command will use your defined `external_url` in `gitlab.rb` and pre-generated SSH key pairs.
-
-    Read more in [additional info for SSH key pairs](#additional-information-for-the-ssh-key-pairs).
+    This command will use your defined `external_url` in `gitlab.rb`
 ### Step 2. Updating the `known_hosts` file of the secondary nodes
@@ -282,29 +280,14 @@ Just omit the first step that sets up the primary node.
 ## Additional information for the SSH key pairs
-When adding a new Geo node, you must provide an SSH public key of the user that
-your GitLab instance runs on (unless changed, should be the user `git`). This
-user will act as a "normal user" who fetches from the primary Geo node.
-
-If for any reason you generate the key using a different name from the default
-`id_rsa`, or you want to generate an extra key only for the repository
-synchronization feature, you can do so, but you have to create/modify your
-`~/.ssh/config` (for the `git` user).
-This is an example on how to change the default key for all remote hosts:
-
-```bash
-Host *                              # Match all remote hosts
-  IdentityFile ~/.ssh/mycustom.key  # The location of your private key
-```
-
-This is how to change it for an specific host:
-
-```bash
-Host example.com                    # The FQDN of the primary Geo node
-  HostName example.com              # The FQDN of the primary Geo node
-  IdentityFile ~/.ssh/mycustom.key  # The location of your private key
-```
+When adding a new **secondary** Geo node, you must provide an SSH public key for
+the system user that your GitLab instance runs as (unless changed, should be the
+user `git`). This user will act as a "normal user" who fetches from the primary
+Geo node.
+Omnibus automatically generates `~git/.ssh/id_rsa` and `~git/.ssh/id_rsa.pub`
+files on secondary Geo nodes. Primaries do not need these files, and you should
+not create them manually.
 ### Upgrading Geo

doc/gitlab-geo/configuration_source.md

@@ -63,32 +63,12 @@ logins opened on all nodes as we will be moving back and forth.
     sudo -i
     ```
-1. (Source install only): Create a new SSH key pair for the primary node. Choose the default location
-   and leave the password blank by hitting 'Enter' three times:
+1. Add this node as the Geo primary by running:
     ```bash
-    sudo -u git -H ssh-keygen -b 4096 -C 'Primary GitLab Geo node'
+    bundle exec rake geo:set_primary_node
     ```
-    Read more in [additional info for SSH key pairs](#additional-information-for-the-ssh-key-pairs).
-
-1. Get the contents of `id_rsa.pub` for the git user:
-
-    ```
-    sudo -u git cat /home/git/.ssh/id_rsa.pub
-    ```
-
-1. Visit the primary node's **Admin Area ➔ Geo Nodes** (`/admin/geo_nodes`) in
-   your browser.
-
-1. Add the primary node by providing its full URL and the public SSH key
-   you created previously. Make sure to check the box 'This is a primary node'
-   when adding it.
-
-    ![Add new primary Geo node](img/geo_nodes_add_new.png)
-
-1. Click the **Add node** button.
-
 ### Step 2. Updating the `known_hosts` file of the secondary nodes
 1. SSH into the **secondary** node and login as root:
@@ -318,7 +298,7 @@ Point your users to the [after setup steps](after_setup.md).
 ## Adding another secondary Geo node
 To add another Geo node in an already Geo configured infrastructure, just follow
-[the steps starting form step 2](#step-2-updating-the-known_hosts-file-of-the-secondary-nodes).
+[the steps starting from step 2](#step-2-updating-the-known_hosts-file-of-the-secondary-nodes).
 Just omit the first step that sets up the primary node.
 ## Additional information for the SSH key pairs

doc/gitlab-geo/disaster-recovery.md

@@ -21,19 +21,25 @@ You must make the changes in the exact specific order:
 1. Take down your primary node (or make sure it will not go up during this
    process or you may lose data)
-2. Wait for any database replication to finish
-3. Promote the Postgres in your secondary node as primary
-4. Log-in to your secondary node with a user with `sudo` permission
-5. Open the interactive rails console: `sudo gitlab-rails console` and execute:
+1. Wait for any database replication to finish
+1. Promote the Postgres in your secondary node as primary
+1. Modify the `gitlab.rb` for both nodes to reflect their new statuses
+1. Log-in to your secondary node with a user with `sudo` permission
+1. **Remove** the Geo SSH client keys (this is very important!):
+
+    ```bash
+    sudo rm ~git/.ssh/id_rsa ~git/.ssh/id_rsa.pub
+    ```
+1. Open the interactive rails console: `sudo gitlab-rails console` and execute:
     * List your primary node and note down it's id:
         ```ruby
         Gitlab::Geo.primary_node
         ```
-    * Turn your primary into a secondary:
+    * Remove the old primary node:
         ```ruby
-        Gitlab::Geo.primary_node.update(primary: false)
+        Gitlab::Geo.primary_node.destroy
         ```
     * List your secondary nodes and note down the id of the one you want to promote:
@@ -51,12 +57,11 @@ You must make the changes in the exact specific order:
         Gitlab::Geo.primary_node.oauth_application.destroy!
         Gitlab::Geo.primary_node.system_hook.destroy!
         ```
-    * And refresh your old primary node to behave correctly as secondary (assuming id is `1`)
-
-        ```ruby
-        GeoNode.find(1).save!
-        ```
     * To exit the interactive console, type: `exit`
-6. Rsync everything in `/var/opt/gitlab/gitlab-rails/uploads` and
+1. Rsync everything in `/var/opt/gitlab/gitlab-rails/uploads` and
    `/var/opt/gitlab/gitlab-rails/shared` from your old node to the new one.
+
+To bring your old primary node back into use as a working secondary, you need to
+run `gitlab-ctl reconfigure` against the node and then follow the
+[setup instructions](README.md) again, as if for a secondary node, from step 3.

lib/system_check/app/git_user_default_ssh_config_check.rb

@@ -10,10 +10,10 @@ class GitUserDefaultSSHConfigCheck < SystemCheck::BaseCheck
       ].freeze
       set_name 'Git user has default SSH configuration?'
-      set_skip_reason 'skipped (git user is not present or configured)'
+      set_skip_reason 'skipped (Geo secondary, or git user is not present / configured)'
       def skip?
-        !home_dir || !File.directory?(home_dir)
+        Gitlab::Geo.secondary? || !home_dir || !File.directory?(home_dir)
       end
       def check?

lib/tasks/geo.rake

@@ -79,33 +79,21 @@ namespace :geo do
   end
   desc 'Make this node the Geo primary'
-  task :set_primary_node, [:ssh_key_filename] => :environment do |_, args|
-    filename = args[:ssh_key_filename]
+  task set_primary_node: :environment do
     abort 'GitLab Geo is not supported with this license. Please contact sales@gitlab.com.' unless Gitlab::Geo.license_allows?
-    abort 'You must specify a filename of an SSH public key' unless filename.present?
     abort 'GitLab Geo primary node already present' if Gitlab::Geo.primary_node.present?
-    public_key = load_ssh_public_key(filename)
-
-    abort "Invalid SSH public key in #{filename}, aborting" unless public_key
-
-    set_primary_geo_node(public_key)
-  end
-
-  def load_ssh_public_key(filename)
-    File.open(filename).read
-  rescue => e
-    puts "Error opening #{filename}: #{e}".color(:red)
-    nil
+    set_primary_geo_node
   end
-  def set_primary_geo_node(public_key)
-    params = { schema: Gitlab.config.gitlab.protocol,
-               host: Gitlab.config.gitlab.host,
-               port: Gitlab.config.gitlab.port,
-               relative_url_root: Gitlab.config.gitlab.relative_url_root,
-               primary: true,
-               geo_node_key_attributes: { key: public_key } }
+  def set_primary_geo_node
+    params = {
+      schema: Gitlab.config.gitlab.protocol,
+      host: Gitlab.config.gitlab.host,
+      port: Gitlab.config.gitlab.port,
+      relative_url_root: Gitlab.config.gitlab.relative_url_root,
+      primary: true
+    }
     node = GeoNode.new(params)
     puts "Saving primary GeoNode with URL #{node.url}".color(:green)

lib/tasks/gitlab/check.rake

@@ -432,7 +432,8 @@ namespace :gitlab do
         SystemCheck::Geo::GeoDatabaseConfiguredCheck,
         SystemCheck::Geo::DatabaseReplicationCheck,
         SystemCheck::Geo::HttpConnectionCheck,
-        SystemCheck::Geo::ClocksSynchronizationCheck
+        SystemCheck::Geo::ClocksSynchronizationCheck,
+        SystemCheck::App::GitUserDefaultSSHConfigCheck
       ]
       SystemCheck.run('Geo', checks)

spec/factories/geo_nodes.rb

@@ -7,6 +7,7 @@
     trait :primary do
       primary true
       port { Gitlab.config.gitlab.port }
+      geo_node_key nil
     end
     trait :current do

spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb

@@ -34,6 +34,15 @@
       it { is_expected.to eq(expected_result) }
     end
+
+    # EE-only
+    it 'skips Geo secondaries' do
+      stub_user
+      stub_home_dir
+      allow(Gitlab::Geo).to receive(:secondary?).and_return(true)
+
+      is_expected.to be_truthy
+    end
   end
   describe '#check?' do

spec/models/geo_node_spec.rb

 require 'spec_helper'
 describe GeoNode, type: :model do
-  subject(:new_node) { create(:geo_node, schema: 'https', host: 'localhost', port: 3000, relative_url_root: 'gitlab', primary: false) }
-  subject(:new_primary_node) { create(:geo_node, schema: 'https', host: 'localhost', port: 3000, relative_url_root: 'gitlab', primary: true) }
-  subject(:empty_node) { described_class.new }
-  subject(:primary_node) { create(:geo_node, :primary) }
-  subject(:node) { create(:geo_node) }
+  let(:new_node) { create(:geo_node, schema: 'https', host: 'localhost', port: 3000, relative_url_root: 'gitlab') }
+  let(:new_primary_node) { create(:geo_node, :primary, schema: 'https', host: 'localhost', port: 3000, relative_url_root: 'gitlab') }
+  let(:empty_node) { described_class.new }
+  let(:primary_node) { create(:geo_node, :primary) }
+  let(:node) { create(:geo_node) }
   let(:dummy_url) { 'https://localhost:3000/gitlab' }
   let(:url_helpers) { Gitlab::Routing.url_helpers }
@@ -19,9 +19,12 @@
     it { is_expected.to have_many(:namespaces).through(:geo_node_namespace_links) }
   end
-  context 'default values' do
-    subject { described_class.new }
+  context 'validations' do
+    it { expect(new_node).to validate_presence_of(:geo_node_key) }
+    it { expect(new_primary_node).not_to validate_presence_of(:geo_node_key) }
+  end
+  context 'default values' do
     let(:gitlab_host) { 'gitlabhost' }
     before do
@@ -29,23 +32,23 @@
     end
     it 'defines a default schema' do
-      expect(subject.schema).to eq('http')
+      expect(empty_node.schema).to eq('http')
     end
     it 'defines a default host' do
-      expect(subject.host).to eq(gitlab_host)
+      expect(empty_node.host).to eq(gitlab_host)
     end
     it 'defines a default port' do
-      expect(subject.port).to eq(80)
+      expect(empty_node.port).to eq(80)
     end
     it 'defines a default relative_url_root' do
-      expect(subject.relative_url_root).to eq('')
+      expect(empty_node.relative_url_root).to eq('')
     end
     it 'defines a default primary flag' do
-      expect(subject.primary).to eq(false)
+      expect(empty_node.primary).to eq(false)
     end
   end
@@ -100,7 +103,7 @@
     let(:new_node) { FactoryGirl.build(:geo_node) }
     it 'expires cache when saved' do
-      expect(new_node).to receive(:expire_cache!)
+      expect(new_node).to receive(:expire_cache!).at_least(:once)
       new_node.save!
     end
@@ -315,4 +318,34 @@
       end
     end
   end
+
+  describe '#geo_node_key' do
+    context 'primary node' do
+      it 'cannot be set' do
+        node = new_primary_node
+
+        expect(node.geo_node_key).to be_nil
+
+        node.geo_node_key = build(:geo_node_key)
+        expect(node).to be_valid
+
+        node.save!
+
+        expect(node.geo_node_key(true)).to be_nil
+      end
+    end
+
+    context 'secondary node' do
+      it 'is automatically set' do
+        node = build(:geo_node, url: 'http://example.com/')
+
+        expect(node.geo_node_key).to be_present
+        expect(node.geo_node_key.title).not_to include('example.com')
+
+        node.save!
+
+        expect(node.geo_node_key.title).to eq('Geo node: http://example.com/')
+      end
+    end
+  end
 end

spec/tasks/geo_rake_spec.rb

@@ -6,32 +6,23 @@
   end
   describe 'set_primary_node task' do
-    let(:ssh_key) { 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDUkxk8m9rVYZ1q4/5xpg3TwTM9QFw3TinPFkyWsiACFKjor3byV6g3vHWTuIS70E7wk2JTXGL0wdrfUG6iQDJuP0BYNxjkluB14nIAfPuXN7V73QY/cqvHogw5o6pPRFD+Szke6FzouNQ70Z/qrM1k7me3e9DMuscMMrMTOR2HLKppNQyP4Jp0WJOyncdWB2NxKXTezy/ZnHv+BdhC0q0JW3huIx9qkBCHio7x8BdyJLMF9KxNYIuCkbP3exs5wgb+qGrjSri6LfAVq8dJ2VYibWxdsUG6iITJF+G4qbcyQjgiMLbxCfNd9bjwmkxSGvFn2EPsAFKzxyAvYFWb/y91 test@host' }
-
     before do
       expect(Gitlab::Geo).to receive(:license_allows?).and_return(true)
       stub_config_setting(protocol: 'https')
     end
     it 'creates a GeoNode' do
-      begin
-        file = Tempfile.new('geo-test-')
-        file.write(ssh_key)
-        path = file.path
-        file.close
-        expect(GeoNode.count).to eq(0)
-        run_rake_task('geo:set_primary_node', path)
-        expect(GeoNode.count).to eq(1)
-        node = GeoNode.first
-        expect(node.schema).to eq('https')
-        expect(node.primary).to be_truthy
-        expect(node.geo_node_key.key).to eq(ssh_key)
-      ensure
-        file.unlink
-      end
+      expect(GeoNode.count).to eq(0)
+
+      run_rake_task('geo:set_primary_node')
+      expect(GeoNode.count).to eq(1)
+      node = GeoNode.first
+      expect(node.schema).to eq('https')
+      expect(node.primary).to be_truthy
+      expect(node.geo_node_key).to be_nil
     end
   end
 end