Remove system_hook from GeoNode

app/models/geo_node.rb

@@ -3,7 +3,6 @@ class GeoNode < ActiveRecord::Base
 
   belongs_to :geo_node_key, dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent
   belongs_to :oauth_application, class_name: 'Doorkeeper::Application', dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent
-  belongs_to :system_hook, dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent
 
   has_many :geo_node_namespace_links
   has_many :namespaces, through: :geo_node_namespace_links
@@ -14,7 +13,7 @@ class GeoNode < ActiveRecord::Base
                  relative_url_root: lambda { Gitlab.config.gitlab.relative_url_root },
                  primary: false
 
-  accepts_nested_attributes_for :geo_node_key, :system_hook
+  accepts_nested_attributes_for :geo_node_key
 
   validates :host, host: true, presence: true, uniqueness: { case_sensitive: false, scope: :port }
   validates :primary, uniqueness: { message: 'node already exists' }, if: :primary
@@ -194,7 +193,6 @@ class GeoNode < ActiveRecord::Base
       update_clone_url
     else
       update_oauth_application!
-      update_system_hook!
     end
   end
 
@@ -217,17 +215,6 @@ class GeoNode < ActiveRecord::Base
     self.oauth_application.redirect_uri = oauth_callback_url
   end
 
-  def update_system_hook!
-    return if self.primary?
-
-    self.build_system_hook if system_hook.nil?
-    self.system_hook.token = SecureRandom.hex(20) unless self.system_hook.token.present?
-    self.system_hook.url = geo_events_url if uri.present?
-    self.system_hook.push_events = false
-    self.system_hook.tag_push_events = false
-    self.system_hook.repository_update_events = true
-  end
-
   def expire_cache!
     Gitlab::Geo.expire_cache!
   end

changelogs/unreleased-ee/tc-geo-remove-system-hooks.yml

+---
+title: Improve copy so users will set up SSH from DB for Geo
+merge_request: 2644
+author:
+type: changed

db/migrate/20170811082658_remove_system_hook_from_geo_nodes.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveSystemHookFromGeoNodes < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def up
+    SystemHook.destroy_all(id: GeoNode.select(:system_hook_id))
+
+    remove_reference :geo_nodes, :system_hook
+  end
+
+  def down
+    add_column :geo_nodes, :system_hook_id, :integer
+  end
+end

db/schema.rb

@@ -683,7 +683,6 @@ ActiveRecord::Schema.define(version: 20170824162758) do
     t.boolean "primary"
     t.integer "geo_node_key_id"
     t.integer "oauth_application_id"
-    t.integer "system_hook_id"
     t.boolean "enabled", default: true, null: false
     t.string "access_key"
     t.string "encrypted_secret_access_key"

spec/ee/spec/migrations/remove_system_hook_from_geo_nodes_spec.rb

+require "spec_helper"
+require Rails.root.join("db", "migrate", "20170811082658_remove_system_hook_from_geo_nodes.rb")
+
+describe RemoveSystemHookFromGeoNodes, :migration do
+  let(:geo_nodes) { table(:geo_nodes) }
+
+  before do
+    allow_any_instance_of(WebHookService).to receive(:execute)
+
+    create(:system_hook)
+    geo_nodes.create! attributes_for(:geo_node, :primary, :current)
+    geo_nodes.create! attributes_for(:geo_node, system_hook_id: create(:system_hook).id)
+  end
+
+  it 'destroy all system hooks for secondary nodes' do
+    expect do
+      migrate!
+    end.to change { SystemHook.count }.by(-1)
+  end
+end

spec/models/geo_node_spec.rb

@@ -49,19 +49,6 @@ describe GeoNode, type: :model do
     end
   end
 
-  context 'system hooks' do
-    it 'primary does not create a system hook' do
-      expect(primary_node.system_hook).to be_nil
-    end
-
-    it 'secondary creates a system hook with repository update events' do
-      hook = new_node.system_hook
-      expect(hook.push_events).to be_falsey
-      expect(hook.tag_push_events).to be_falsey
-      expect(hook.repository_update_events).to be_truthy
-    end
-  end
-
   context 'prevent locking yourself out' do
     subject do
       GeoNode.new(host: Gitlab.config.gitlab.host,
@@ -97,19 +84,6 @@ describe GeoNode, type: :model do
         expect(node.oauth_application).to be_persisted
       end
 
-      it 'has a system_hook if it is a secondary node' do
-        expect(node.system_hook).to be_present
-      end
-
-      it 'generated system_hook has required attributes' do
-        expect(node.system_hook.url).to be_present
-        expect(node.system_hook.url).to eq(node.geo_events_url)
-        expect(node.system_hook.token).to be_present
-        expect(node.system_hook.push_events).to be_falsey
-        expect(node.system_hook.tag_push_events).to be_falsey
-        expect(node.system_hook.repository_update_events).to be_truthy
-      end
-
       context 'when is a primary node' do
         it 'has no oauth_application' do
           expect(primary_node.oauth_application).not_to be_present