Merge branch '1144-allow-admins-to-disable-push-mirrors' into 'master'

Adds admin application setting to globably disable push mirrors.

Closes #1144

See merge request gitlab-org/gitlab-ee!3130

app/controllers/projects/mirrors_controller.rb

@@ -75,17 +75,36 @@ class Projects::MirrorsController < Projects::ApplicationController
     @remote_mirror = @project.remote_mirrors.first_or_initialize
   end
 
+  def remote_mirror_attributes
+    { remote_mirrors_attributes: %i[url id enabled] }
+  end
+
+  def mirror_params_attributes
+    attributes = [
+      :mirror,
+      :import_url,
+      :username_only_import_url,
+      :mirror_user_id,
+      :mirror_trigger_builds,
+
+      import_data_attributes: %i[
+        id
+        auth_method
+        password
+        ssh_known_hosts
+        regenerate_ssh_private_key
+      ]
+    ]
+
+    if can?(current_user, :admin_remote_mirror, project)
+      attributes << remote_mirror_attributes
+    end
+
+    attributes
+  end
+
   def mirror_params
-    params.require(:project)
-      .permit(
-        :mirror,
-        :import_url,
-        :username_only_import_url,
-        :mirror_user_id,
-        :mirror_trigger_builds,
-        import_data_attributes: [:id, :auth_method, :password, :ssh_known_hosts, :regenerate_ssh_private_key],
-        remote_mirrors_attributes: [:url, :id, :enabled]
-      )
+    params.require(:project).permit(mirror_params_attributes)
   end
 
   def safe_mirror_params

app/models/remote_mirror.rb

@@ -17,6 +17,7 @@ class RemoteMirror < ActiveRecord::Base
 
   validate  :url_availability, if: -> (mirror) { mirror.url_changed? || mirror.enabled? }
 
+  after_save :set_override_remote_mirror_available, unless: -> { Gitlab::CurrentSettings.current_application_settings.remote_mirror_available }
   after_save :refresh_remote, if: :mirror_url_changed?
   after_update :reset_fields, if: :mirror_url_changed?
   after_destroy :remove_remote
@@ -85,6 +86,7 @@ class RemoteMirror < ActiveRecord::Base
 
   def enabled
     return false unless project && super
+    return false unless project.remote_mirror_available?
     return false unless project.repository_exists?
     return false if project.pending_delete?
 
@@ -147,6 +149,12 @@ class RemoteMirror < ActiveRecord::Base
     )
   end
 
+  def set_override_remote_mirror_available
+    enabled = read_attribute(:enabled)
+
+    project.update(remote_mirror_available_overridden: enabled)
+  end
+
   def refresh_remote
     return unless project
 

app/services/git_push_service.rb

@@ -126,7 +126,7 @@ class GitPushService < BaseService
   protected
 
   def update_remote_mirrors
-    return if @project.remote_mirrors.empty?
+    return unless @project.has_remote_mirror?
 
     @project.mark_stuck_remote_mirrors_as_failed!
     @project.update_remote_mirrors

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

@@ -126,7 +126,9 @@
               Enabling this will only make licensed EE features available to projects if the project namespace's plan
               includes the feature or if the project is public.
 
-  = render partial: 'repository_mirrors_form', locals: { f: f }
+  - if License.feature_available?(:repository_mirrors)
+    = render partial: 'repository_mirrors_form', locals: { f: f }
+    = render partial: 'repository_remote_mirrors_form', locals: { f: f }
 
   %fieldset
     %legend Sign-up Restrictions

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

+%fieldset
+  %legend Repository Remote mirror settings
+  .form-group
+    = f.label :remote_mirror_available, 'Enable remote mirror configuration', class: 'control-label col-sm-2'
+    .col-sm-10
+      .checkbox
+        = f.label :remote_mirror_available do
+          = f.check_box :remote_mirror_available
+          Allow remote mirrors to be setup for projects
+      %span.help-block
+        If disabled, only admins will be able to setup remote mirrors in projects.
+        = link_to icon('question-circle'), help_page_path('workflow/repository_mirroring', anchor: 'pushing-to-a-remote-repository')

app/views/projects/mirrors/_show.html.haml

@@ -3,4 +3,5 @@
 
 - if @project.feature_available?(:repository_mirrors)
   = render 'projects/mirrors/pull'
-  = render 'projects/mirrors/push'
+  - if can?(current_user, :admin_remote_mirror, @project)
+    = render 'projects/mirrors/push'

changelogs/unreleased-ee/1144-allow-admins-to-disable-push-mirrors.yml

+---
+title: Allow admins to globally disable all remote mirrors from application settings
+  page.
+merge_request:
+author:
+type: added

db/migrate/20171017125928_add_remote_mirror_available_to_application_settings.rb

+class AddRemoteMirrorAvailableToApplicationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:application_settings, :remote_mirror_available, :boolean, default: true, allow_null: false)
+  end
+
+  def down
+    remove_column(:application_settings, :remote_mirror_available)
+  end
+end

db/migrate/20171017130239_add_remote_mirror_available_overridden_to_projects.rb

+class AddRemoteMirrorAvailableOverriddenToProjects < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column(:projects, :remote_mirror_available_overridden, :boolean)
+  end
+
+  def down
+    remove_column(:projects, :remote_mirror_available_overridden)
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171010140746) do
+ActiveRecord::Schema.define(version: 20171017130239) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -157,6 +157,7 @@ ActiveRecord::Schema.define(version: 20171010140746) do
     t.boolean "hashed_storage_enabled", default: false, null: false
     t.boolean "project_export_enabled", default: true, null: false
     t.boolean "auto_devops_enabled", default: false, null: false
+    t.boolean "remote_mirror_available", default: true, null: false
   end
 
   create_table "approvals", force: :cascade do |t|
@@ -1603,6 +1604,7 @@ ActiveRecord::Schema.define(version: 20171010140746) do
     t.boolean "disable_overriding_approvers_per_merge_request"
     t.integer "storage_version", limit: 2
     t.boolean "resolve_outdated_diff_discussions"
+    t.boolean "remote_mirror_available_overridden"
   end
 
   add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree

ee/app/helpers/ee/application_settings_helper.rb

@@ -22,7 +22,8 @@ module EE
         :slack_app_id,
         :slack_app_secret,
         :slack_app_verification_token,
-        :allow_group_owners_to_manage_ldap
+        :allow_group_owners_to_manage_ldap,
+        :remote_mirror_available
       ]
     end
 

ee/app/models/ee/application_setting.rb

@@ -40,7 +40,8 @@ module EE
           mirror_max_delay: Settings.gitlab['mirror_max_delay'],
           mirror_max_capacity: Settings.gitlab['mirror_max_capacity'],
           mirror_capacity_threshold: Settings.gitlab['mirror_capacity_threshold'],
-          allow_group_owners_to_manage_ldap: true
+          allow_group_owners_to_manage_ldap: true,
+          remote_mirror_available: true
         )
       end
     end

ee/app/models/ee/project.rb

@@ -132,7 +132,9 @@ module EE
     end
 
     def has_remote_mirror?
-      feature_available?(:repository_mirrors) && remote_mirrors.enabled.exists?
+      feature_available?(:repository_mirrors) &&
+        remote_mirror_available? &&
+        remote_mirrors.enabled.exists?
     end
 
     def updating_remote_mirror?
@@ -140,7 +142,7 @@ module EE
     end
 
     def update_remote_mirrors
-      return unless feature_available?(:repository_mirrors)
+      return unless feature_available?(:repository_mirrors) && remote_mirror_available?
 
       remote_mirrors.enabled.each(&:sync)
     end
@@ -463,6 +465,11 @@ module EE
       @disabled_services
     end
 
+    def remote_mirror_available?
+      remote_mirror_available_overridden ||
+        current_application_settings.remote_mirror_available
+    end
+
     private
 
     def licensed_feature_available?(feature)

ee/app/policies/ee/project_policy.rb

@@ -20,6 +20,11 @@ module EE
         !PushRule.global&.reject_unsigned_commits
       end
 
+      with_scope :global
+      condition(:remote_mirror_available) do
+        ::Gitlab::CurrentSettings.current_application_settings.remote_mirror_available
+      end
+
       rule { admin }.enable :change_repository_storage
 
       rule { support_bot }.enable :guest_access
@@ -49,6 +54,8 @@ module EE
 
       rule { can?(:developer_access) }.enable :admin_board
 
+      rule { (remote_mirror_available & can?(:admin_project)) | admin }.enable :admin_remote_mirror
+
       rule { deploy_board_disabled & ~is_development }.prevent :read_deploy_board
 
       rule { can?(:master_access) }.policy do

lib/gitlab/import_export/import_export.yml

@@ -105,6 +105,7 @@ excluded_attributes:
     - :mirror_user_id
     - :mirror_trigger_builds
     - :storage_version
+    - :remote_mirror_available_overridden
   snippets:
     - :expired_at
   merge_request_diff:

spec/controllers/projects/mirrors_controller_spec.rb

@@ -4,6 +4,37 @@ describe Projects::MirrorsController do
   include ReactiveCachingHelpers
 
   describe 'setting up a remote mirror' do
+    set(:project) { create(:project, :repository) }
+    let(:url) { 'http://foo.com' }
+
+    context 'when remote mirrors are disabled' do
+      before do
+        stub_application_setting(remote_mirror_available: false)
+      end
+
+      context 'when user is admin' do
+        let(:admin) { create(:user, :admin) }
+
+        it 'creates a new remote mirror' do
+          sign_in(admin)
+
+          expect do
+            do_put(project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => url } })
+          end.to change { RemoteMirror.count }.to(1)
+        end
+      end
+
+      context 'when user is not admin' do
+        it 'does not create a new remote mirror' do
+          sign_in(project.owner)
+
+          expect do
+            do_put(project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => url } })
+          end.not_to change { RemoteMirror.count }
+        end
+      end
+    end
+
     context 'when the current project is a mirror' do
       let(:project) { create(:project, :repository, :mirror) }
 
@@ -38,7 +69,6 @@ describe Projects::MirrorsController do
 
     context 'when the current project is not a mirror' do
       it 'allows to create a remote mirror' do
-        project = create(:project, :repository)
         sign_in(project.owner)
 
         expect do
@@ -48,7 +78,6 @@ describe Projects::MirrorsController do
     end
 
     context 'when the current project has a remote mirror' do
-      let(:project) { create(:project, :repository) }
       let(:remote_mirror) { project.remote_mirrors.create!(enabled: 1, url: 'http://local.dev') }
 
       before do
@@ -117,7 +146,7 @@ describe Projects::MirrorsController do
     end
   end
 
-  describe 'forcing an update' do
+  describe 'forcing an update on a pull mirror' do
     it 'forces update' do
       expect_any_instance_of(EE::Project).to receive(:force_import_job!)
 
@@ -128,6 +157,25 @@ describe Projects::MirrorsController do
     end
   end
 
+  describe 'forcing an update on a push mirror' do
+    context 'when remote mirrors are disabled' do
+      let(:project) { create(:project, :repository, :remote_mirror) }
+
+      before do
+        stub_application_setting(remote_mirror_available: false)
+        sign_in(project.owner)
+      end
+
+      it 'updates now when overridden' do
+        project.update(remote_mirror_available_overridden: true)
+
+        expect_any_instance_of(EE::Project).to receive(:update_remote_mirrors)
+
+        put :update_now, { namespace_id: project.namespace.to_param, project_id: project.to_param, sync_remote: 1 }
+      end
+    end
+  end
+
   describe '#update' do
     let(:project) { create(:project, :repository, :mirror, :remote_mirror) }
 

spec/ee/spec/models/ee/project_spec.rb

@@ -277,6 +277,15 @@ describe Project do
       update_remote_mirrors
     end
 
+    it 'does nothing when remote mirror is disabled globally and not overridden' do
+      stub_application_setting(remote_mirror_available: false)
+      project.remote_mirror_available_overridden = false
+
+      expect_any_instance_of(RemoteMirror).not_to receive(:sync)
+
+      update_remote_mirrors
+    end
+
     it 'does nothing when unlicensed' do
       stub_licensed_features(repository_mirrors: false)
 
@@ -824,6 +833,32 @@ describe Project do
     end
   end
 
+  describe '#remote_mirror_available?' do
+    let(:project) { create(:project) }
+
+    context 'when remote mirror global setting is enabled' do
+      it 'returns true' do
+        expect(project.remote_mirror_available?).to be(true)
+      end
+    end
+
+    context 'when remote mirror global setting is disabled' do
+      before do
+        stub_application_setting(remote_mirror_available: false)
+      end
+
+      it 'returns true when overridden' do
+        project.remote_mirror_available_overridden = true
+
+        expect(project.remote_mirror_available?).to be(true)
+      end
+
+      it 'returns false when not overridden' do
+        expect(project.remote_mirror_available?).to be(false)
+      end
+    end
+  end
+
   describe '#username_only_import_url' do
     where(:import_url, :username, :expected_import_url) do
       '' | 'foo' | ''

spec/ee/spec/policies/project_policy_spec.rb

+require 'spec_helper'
+
+describe ProjectPolicy do
+  set(:owner) { create(:user) }
+  set(:admin) { create(:admin) }
+  set(:developer) { create(:user) }
+  let(:project) { create(:project, :public, namespace: owner.namespace) }
+
+  before do
+    project.add_developer(developer)
+  end
+
+  context 'admin_remote_mirror' do
+    context 'with remote mirror setting enabled' do
+      context 'with admin' do
+        subject do
+          described_class.new(admin, project)
+        end
+
+        it do
+          is_expected.to be_allowed(:admin_remote_mirror)
+        end
+      end
+
+      context 'with owner' do
+        subject do
+          described_class.new(owner, project)
+        end
+
+        it do
+          is_expected.to be_allowed(:admin_remote_mirror)
+        end
+      end
+
+      context 'with developer' do
+        subject do
+          described_class.new(developer, project)
+        end
+
+        it do
+          is_expected.to be_disallowed(:admin_remote_mirror)
+        end
+      end
+    end
+
+    context 'with remote mirror setting disabled' do
+      before do
+        stub_application_setting(remote_mirror_available: false)
+      end
+
+      context 'with admin' do
+        subject do
+          described_class.new(admin, project)
+        end
+
+        it do
+          is_expected.to be_allowed(:admin_remote_mirror)
+        end
+      end
+
+      context 'with owner' do
+        subject do
+          described_class.new(owner, project)
+        end
+
+        it do
+          is_expected.to be_disallowed(:admin_remote_mirror)
+        end
+      end
+    end
+  end
+end

spec/models/remote_mirror_spec.rb

@@ -120,14 +120,6 @@ describe RemoteMirror do
       end
     end
 
-    context 'without project' do
-      it 'returns nil' do
-        allow_any_instance_of(described_class).to receive(:project).and_return(nil)
-
-        expect(remote_mirror.sync).to be_nil
-      end
-    end
-
     context 'as a Geo secondary' do
       it 'returns nil' do
         allow(Gitlab::Geo).to receive(:secondary?).and_return(true)

spec/services/git_push_service_spec.rb

@@ -21,17 +21,62 @@ describe GitPushService do
       described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
     end
 
-    it 'fails stuck remote mirrors' do
-      allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors)
-      expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
+    context 'when remote mirror feature is enabled' do
+      it 'fails stuck remote mirrors' do
+        allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors)
+        expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
 
-      subject.execute
+        subject.execute
+      end
+
+      it 'updates remote mirrors' do
+        expect(project).to receive(:update_remote_mirrors)
+
+        subject.execute
+      end
     end
 
-    it 'updates remote mirrors' do
-      expect(project).to receive(:update_remote_mirrors)
+    context 'when remote mirror feature is disabled' do
+      before do
+        stub_application_setting(remote_mirror_available: false)
+      end
+
+      context 'with remote mirrors global setting overridden' do
+        before do
+          project.remote_mirror_available_overridden = true
+        end
+
+        it 'fails stuck remote mirrors' do
+          allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors)
+          expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
+
+          subject.execute
+        end
+
+        it 'updates remote mirrors' do
+          expect(project).to receive(:update_remote_mirrors)
 
-      subject.execute
+          subject.execute
+        end
+      end
+
+      context 'without remote mirrors global setting overridden' do
+        before do
+          project.remote_mirror_available_overridden = false
+        end
+
+        it 'does not fails stuck remote mirrors' do
+          expect(project).not_to receive(:mark_stuck_remote_mirrors_as_failed!)
+
+          subject.execute
+        end
+
+        it 'does not updates remote mirrors' do
+          expect(project).not_to receive(:update_remote_mirrors)
+
+          subject.execute
+        end
+      end
     end
   end