Merge branch 'issue_547' into 'master'

Bugfix: Remote config URL was not updated when mirror credentials were updated

The `after_save` callback was looking for changes only in the URL
without credentials, this was happening because we store the credentials
in a separate field.

REF: #547 

See merge request !426

app/models/remote_mirror.rb

@@ -2,16 +2,19 @@
 #
 # Table name: remote_mirrors
 #
-#  id                        :integer          not null, primary key
-#  project_id                :integer
-#  url                       :string
-#  last_update_at            :datetime
-#  last_error                :string
-#  created_at                :datetime         not null
-#  updated_at                :datetime         not null
-#  last_successful_update_at :datetime
-#  update_status             :string
-#  enabled                   :boolean          default(TRUE)
+#  id                         :integer          not null, primary key
+#  project_id                 :integer
+#  url                        :string
+#  enabled                    :boolean          default(TRUE)
+#  update_status              :string
+#  last_update_at             :datetime
+#  last_successful_update_at  :datetime
+#  last_error                 :string
+#  encrypted_credentials      :text
+#  encrypted_credentials_iv   :string
+#  encrypted_credentials_salt :string
+#  created_at                 :datetime         not null
+#  updated_at                 :datetime         not null
 #
 
 class RemoteMirror < ActiveRecord::Base
@@ -28,8 +31,8 @@ class RemoteMirror < ActiveRecord::Base
   validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true }
   validate  :url_availability, if: :url_changed?
 
-  after_save :refresh_remote, if: :url_changed?
-  after_update :reset_fields, if: :url_changed?
+  after_save :refresh_remote, if: :mirror_url_changed?
+  after_update :reset_fields, if: :mirror_url_changed?
   after_destroy :remove_remote
 
   scope :enabled, -> { where(enabled: true) }
@@ -155,4 +158,8 @@ class RemoteMirror < ActiveRecord::Base
   def remove_remote
     project.repository.remove_remote(ref_name)
   end
+
+  def mirror_url_changed?
+    url_changed? || encrypted_credentials_changed?
+  end
 end

app/models/repository.rb

@@ -205,6 +205,10 @@ class Repository
     end
   end
 
+  def config
+    raw_repository.rugged.config
+  end
+
   def add_remote(name, url)
     raw_repository.remote_add(name, url)
   rescue Rugged::ConfigError
@@ -219,12 +223,10 @@ class Repository
   end
 
   def set_remote_as_mirror(name)
-    remote_config = raw_repository.rugged.config
-
     # This is used by Gitlab Geo to define repository as equivalent as "git clone --mirror"
-    remote_config["remote.#{name}.fetch"] = 'refs/*:refs/*'
-    remote_config["remote.#{name}.mirror"] = true
-    remote_config["remote.#{name}.prune"] = true
+    config["remote.#{name}.fetch"] = 'refs/*:refs/*'
+    config["remote.#{name}.mirror"] = true
+    config["remote.#{name}.prune"] = true
   end
 
   def fetch_remote(remote, forced: false, no_tags: false)

spec/models/remote_mirror_spec.rb

+# == Schema Information
+#
+# Table name: remote_mirrors
+#
+#  id                         :integer          not null, primary key
+#  project_id                 :integer
+#  url                        :string
+#  enabled                    :boolean          default(TRUE)
+#  update_status              :string
+#  last_update_at             :datetime
+#  last_successful_update_at  :datetime
+#  last_error                 :string
+#  encrypted_credentials      :text
+#  encrypted_credentials_iv   :string
+#  encrypted_credentials_salt :string
+#  created_at                 :datetime         not null
+#  updated_at                 :datetime         not null
+#
+
 require 'rails_helper'
 
 describe RemoteMirror do
@@ -43,6 +62,15 @@ describe RemoteMirror do
         expect(mirror.url).to eq('http://foo:bar@test.com')
         expect(mirror.credentials).to eq({ user: 'foo', password: 'bar' })
       end
+
+      it 'should update the remote config if credentials changed' do
+        mirror = create_mirror_with_url('http://foo:bar@test.com')
+        repo = mirror.project.repository
+
+        mirror.update_attribute(:url, 'http://foo:baz@test.com')
+
+        expect(repo.config["remote.#{mirror.ref_name}.url"]).to eq('http://foo:baz@test.com')
+      end
     end
   end