Introduce namespace license checks for Push Rules (EES)

app/controllers/admin/push_rules_controller.rb

 class Admin::PushRulesController < Admin::ApplicationController
+  before_action :check_push_rules_available!
   before_action :push_rule
 
   respond_to :html
@@ -18,6 +19,10 @@ class Admin::PushRulesController < Admin::ApplicationController
 
   private
 
+  def check_push_rules_available!
+    render_404 unless License.feature_available?(:push_rules)
+  end
+
   def push_rule_params
     params.require(:push_rule).permit(:deny_delete_tag, :delete_branch_regex,
       :commit_message_regex, :force_push_regex, :author_email_regex, :member_check,

app/controllers/ee/projects/settings/repository_controller.rb

@@ -5,18 +5,19 @@ module EE
         extend ActiveSupport::Concern
 
         prepended do
+          before_action :push_rule, only: [:show]
           before_action :remote_mirror, only: [:show]
         end
 
-        def show
-          super
+        private
+
+        def push_rule
+          return unless project.feature_available?(:push_rules)
 
           project.create_push_rule unless project.push_rule
           @push_rule = project.push_rule
         end
 
-        private
-
         def remote_mirror
           @remote_mirror = @project.remote_mirrors.first_or_initialize
         end

app/controllers/projects/push_rules_controller.rb

@@ -3,6 +3,7 @@ class Projects::PushRulesController < Projects::ApplicationController
   
   # Authorize
   before_action :authorize_admin_project!
+  before_action :check_push_rules_available!
 
   respond_to :html
 

app/models/ee/project.rb

@@ -25,7 +25,7 @@ module EE
       belongs_to :mirror_user, foreign_key: 'mirror_user_id', class_name: 'User'
 
       has_one :mirror_data, dependent: :delete, autosave: true, class_name: 'ProjectMirrorData'
-      has_one :push_rule, dependent: :destroy
+      has_one :push_rule, ->(project) { project&.feature_available?(:push_rules) ? all : none }, dependent: :destroy
       has_one :index_status, dependent: :destroy
       has_one :jenkins_service, dependent: :destroy
       has_one :jenkins_deprecated_service, dependent: :destroy

app/models/license.rb

@@ -13,6 +13,7 @@ class License < ActiveRecord::Base
   MERGE_REQUEST_REBASE_FEATURE = 'GitLab_MergeRequestRebase'.freeze
   MERGE_REQUEST_SQUASH_FEATURE = 'GitLab_MergeRequestSquash'.freeze
   OBJECT_STORAGE_FEATURE = 'GitLab_ObjectStorage'.freeze
+  PUSH_RULES_FEATURE = 'GitLab_PushRules'.freeze
   RELATED_ISSUES_FEATURE = 'RelatedIssues'.freeze
   SERVICE_DESK_FEATURE = 'GitLab_ServiceDesk'.freeze
 
@@ -32,7 +33,8 @@ class License < ActiveRecord::Base
     file_lock: FILE_LOCK_FEATURE,
     issue_weights: ISSUE_WEIGHTS_FEATURE,
     merge_request_rebase: MERGE_REQUEST_REBASE_FEATURE,
-    merge_request_squash: MERGE_REQUEST_SQUASH_FEATURE
+    merge_request_squash: MERGE_REQUEST_SQUASH_FEATURE,
+    push_rules: PUSH_RULES_FEATURE
   }.freeze
 
   STARTER_PLAN = 'starter'.freeze
@@ -48,6 +50,7 @@ class License < ActiveRecord::Base
     { ISSUE_WEIGHTS_FEATURE => 1 },
     { MERGE_REQUEST_REBASE_FEATURE => 1 },
     { MERGE_REQUEST_SQUASH_FEATURE => 1 },
+    { PUSH_RULES_FEATURE => 1 },
     { RELATED_ISSUES_FEATURE => 1 }
   ].freeze
 
@@ -86,6 +89,7 @@ class License < ActiveRecord::Base
     { MERGE_REQUEST_REBASE_FEATURE => 1 },
     { MERGE_REQUEST_SQUASH_FEATURE => 1 },
     { OBJECT_STORAGE_FEATURE => 1 },
+    { PUSH_RULES_FEATURE => 1 },
     { SERVICE_DESK_FEATURE => 1 }
   ].freeze
 

app/services/merge_requests/merge_service.rb

@@ -44,6 +44,7 @@ module MergeRequests
       @merge_request = merge_request
 
       return true if project.merge_requests_ff_only_enabled
+      return true unless project.feature_available?(:push_rules)
 
       push_rule = merge_request.project.push_rule
       return true unless push_rule

app/services/projects/create_service.rb

@@ -164,6 +164,8 @@ module Projects
     end
 
     def create_predefined_push_rule
+      return unless project.feature_available?(:push_rules)
+
       predefined_push_rule = PushRule.find_by(is_sample: true)
 
       if predefined_push_rule

app/views/layouts/nav/_admin.html.haml

@@ -34,23 +34,10 @@
           Abuse Reports
           %span.badge.count= number_with_delimiter(AbuseReport.count(:all))
 
-    = nav_link(controller: :licenses) do
-      = link_to admin_license_path, title: 'License' do
-        %span
-          License
-
     - if akismet_enabled?
       = nav_link(controller: :spam_logs) do
         = link_to admin_spam_logs_path, title: "Spam Logs" do
           %span
             Spam Logs
 
-    = nav_link(controller: :push_rules) do
-      = link_to admin_push_rule_path, title: 'Push Rules' do
-        %span
-          Push Rules
-
-    = nav_link(controller: :geo_nodes) do
-      = link_to admin_geo_nodes_path, title: 'Geo Nodes' do
-        %span
-          Geo Nodes
+    = render 'layouts/nav/admin_ee'

app/views/layouts/nav/_admin_ee.html.haml

+= nav_link(controller: :licenses) do
+  = link_to admin_license_path, title: 'License' do
+    %span
+      License
+
+- if License.feature_available?(:push_rules)
+  = nav_link(controller: :push_rules) do
+    = link_to admin_push_rule_path, title: 'Push Rules' do
+      %span
+        Push Rules
+
+= nav_link(controller: :geo_nodes) do
+  = link_to admin_geo_nodes_path, title: 'Geo Nodes' do
+    %span
+      Geo Nodes

app/views/projects/push_rules/_index.html.haml

+- return unless @project.feature_available?(:push_rules)
+
 - expanded = Rails.env.test?
 %section.settings
   .settings-header

changelogs/unreleased-ee/2573-namespace-license-push-rules.yml

+---
+title: Introduce namespace license checks for Push Rules (EES)
+merge_request: 2335
+author:

lib/api/project_push_rule.rb

@@ -2,6 +2,7 @@ module API
   class ProjectPushRule < Grape::API
     before { authenticate! }
     before { authorize_admin_project }
+    before { check_project_feature_available!(:push_rules) }
 
     params do
       requires :id, type: String, desc: 'The ID of a project'

lib/api/v3/project_git_hook.rb

@@ -3,6 +3,7 @@ module API
     class ProjectGitHook < Grape::API
       before { authenticate! }
       before { authorize_admin_project }
+      before { check_project_feature_available!(:push_rules) }
 
       DEPRECATION_MESSAGE = 'This endpoint is deprecated, replaced with push_rules, and will be removed in GitLab 9.0.'.freeze
 

lib/api/v3/project_push_rule.rb

@@ -3,6 +3,7 @@ module API
     class ProjectPushRule < Grape::API
       before { authenticate! }
       before { authorize_admin_project }
+      before { check_project_feature_available!(:push_rules) }
 
       params do
         requires :id, type: String, desc: 'The ID of a project'

lib/ee/api/helpers.rb

@@ -9,6 +9,10 @@ module EE
 
         user
       end
+
+      def check_project_feature_available!(feature)
+        not_found! unless user_project.feature_available?(feature)
+      end
     end
   end
 end

lib/gitlab/checks/change_access.rb

@@ -142,7 +142,7 @@ module Gitlab
       end
 
       def push_rule_check
-        return unless @newrev && @oldrev
+        return unless @newrev && @oldrev && project.feature_available?(:push_rules)
 
         push_rule = project.push_rule
 

spec/controllers/admin/push_rules_controller_spec.rb

@@ -8,17 +8,52 @@ describe Admin::PushRulesController do
   end
 
   describe '#update' do
-    it 'updates sample push rule' do
-      params =
-        { deny_delete_tag: true, delete_branch_regex: "any", commit_message_regex: "any",
-          force_push_regex: "any", author_email_regex: "any", member_check: true, file_name_regex: "any",
-          max_file_size: "0", prevent_secrets: true }
+    let(:params) do
+      {
+        deny_delete_tag: true, delete_branch_regex: "any", commit_message_regex: "any",
+        force_push_regex: "any", author_email_regex: "any", member_check: true, file_name_regex: "any",
+        max_file_size: "0", prevent_secrets: true
+      }
+    end
 
+    it 'updates sample push rule' do
       expect_any_instance_of(PushRule).to receive(:update_attributes).with(params)
 
       patch :update, push_rule: params
 
       expect(response).to redirect_to(admin_push_rule_path)
     end
+
+    context 'push rules unlicensed' do
+      before do
+        stub_licensed_features(push_rules: false)
+      end
+
+      it 'returns 404' do
+        patch :update, push_rule: params
+
+        expect(response).to have_http_status(404)
+      end
+    end
+  end
+
+  describe '#show' do
+    it 'returns 200' do
+      get :show
+
+      expect(response).to have_http_status(200)
+    end
+
+    context 'push rules unlicensed' do
+      before do
+        stub_licensed_features(push_rules: false)
+      end
+
+      it 'returns 404' do
+        get :show
+
+        expect(response).to have_http_status(404)
+      end
+    end
   end
 end

spec/controllers/projects/push_rules_controller_spec.rb

+require 'spec_helper'
+
+describe Projects::PushRulesController do
+  let(:project) { create(:empty_project, push_rule: create(:push_rule, prevent_secrets: false)) }
+  let(:user) { create(:user) }
+
+  before do
+    project.add_master(user)
+
+    sign_in(user)
+  end
+  
+  describe '#update' do
+    def do_update
+      patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { prevent_secrets: true }
+    end
+
+    it 'updates the push rule' do
+      do_update
+
+      expect(response).to have_http_status(302)
+      expect(project.push_rule(true).prevent_secrets).to be_truthy
+    end
+
+    context 'push rules unlicensed' do
+      before do
+        stub_licensed_features(push_rules: false)
+      end
+
+      it 'returns 404' do
+        do_update
+
+        expect(response).to have_http_status(404)
+      end
+    end
+  end
+end

spec/controllers/projects/settings/repository_controller_ee_spec.rb

+require 'spec_helper'
+
+describe Projects::Settings::RepositoryController do
+  let(:project) { create(:project_empty_repo, :public) }
+  let(:user) { create(:user) }
+
+  before do
+    project.add_master(user)
+    sign_in(user)
+  end
+
+  describe 'GET show' do
+    context 'push rule' do
+      subject(:push_rule) { assigns(:push_rule) }
+
+      it 'is created' do
+        get :show, namespace_id: project.namespace, project_id: project
+
+        is_expected.to be_persisted
+      end
+
+      context 'unlicensed' do
+        before do
+          stub_licensed_features(push_rules: false)
+        end
+
+        it 'is not created' do
+          get :show, namespace_id: project.namespace, project_id: project
+
+          is_expected.to be_nil
+        end
+      end
+    end
+  end
+end

spec/features/projects/settings/ee/push_rules_settings_spec.rb

@@ -15,19 +15,33 @@ describe 'Project settings > [EE] repository', feature: true do
     let(:commit_message) { 'Required part of every message' }
     let(:input_id) { 'push_rule_commit_message_regex' }
 
-    before do
-      visit namespace_project_settings_repository_path(project.namespace, project)
+    context 'push rules licensed' do
+      before do
+        visit namespace_project_settings_repository_path(project.namespace, project)
 
-      fill_in input_id, with: commit_message
-      click_button 'Save Push Rules'
-    end
+        fill_in input_id, with: commit_message
+        click_button 'Save Push Rules'
+      end
+
+      it 'displays the new value in the form' do
+        expect(find("##{input_id}").value).to eq commit_message
+      end
 
-    it 'displays the new value in the form' do
-      expect(find("##{input_id}").value).to eq commit_message
+      it 'saves the new value' do
+        expect(project.push_rule.commit_message_regex).to eq commit_message
+      end
     end
 
-    it 'saves the new value' do
-      expect(project.push_rule.commit_message_regex).to eq commit_message
+    context 'push rules unlicensed' do
+      before do
+        stub_licensed_features(push_rules: false)
+
+        visit namespace_project_settings_repository_path(project.namespace, project)
+      end
+
+      it 'hides push rule settings' do
+        expect(page).not_to have_content('Push Rules')
+      end
     end
   end
 

spec/lib/gitlab/checks/change_access_spec.rb

@@ -165,6 +165,14 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
     end
 
     context 'push rules checks' do
+      shared_examples 'check ignored when push rule unlicensed' do
+        before do
+          stub_licensed_features(push_rules: false)
+        end
+
+        it { is_expected.to be_truthy }
+      end
+
       let(:project) { create(:project, :public, push_rule: push_rule) }
 
       before do
@@ -183,6 +191,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
           project.add_master(user)
         end
 
+        it_behaves_like 'check ignored when push rule unlicensed'
+
         it 'returns an error if the rule denies tag deletion' do
           expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You cannot delete a tag')
         end
@@ -199,6 +209,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
       context 'commit message rules' do
         let(:push_rule) { create(:push_rule, :commit_message) }
 
+        it_behaves_like 'check ignored when push rule unlicensed'
+
         it 'returns an error if the rule fails' do
           expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'")
         end
@@ -212,6 +224,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
           allow_any_instance_of(Commit).to receive(:author_email).and_return('mike@valid.com')
         end
 
+        it_behaves_like 'check ignored when push rule unlicensed'
+
         it 'returns an error if the rule fails for the committer' do
           allow_any_instance_of(Commit).to receive(:committer_email).and_return('ana@invalid.com')
 
@@ -233,6 +247,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
           allow_any_instance_of(Commit).to receive(:author_email).and_return('some@mail.com')
         end
 
+        it_behaves_like 'check ignored when push rule unlicensed'
+
         it 'returns an error if the commit author is not a GitLab member' do
           expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Author 'some@mail.com' is not a member of team")
         end
@@ -243,6 +259,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
         context 'file name regex check' do
           let(:push_rule) { create(:push_rule, file_name_regex: 'READ*') }
 
+          it_behaves_like 'check ignored when push rule unlicensed'
+
           it "returns an error if a new or renamed filed doesn't match the file name regex" do
             expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "File name README was blacklisted by the pattern READ*.")
           end
@@ -251,6 +269,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
         context 'blacklisted files check' do
           let(:push_rule) { create(:push_rule, prevent_secrets: true) }
 
+          it_behaves_like 'check ignored when push rule unlicensed'
+
           it "returns true if there is no blacklisted files" do
             new_rev = nil
 
@@ -305,6 +325,8 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
           allow_any_instance_of(Blob).to receive(:size).and_return(2.megabytes)
         end
 
+        it_behaves_like 'check ignored when push rule unlicensed'
+
         it 'returns an error if file exceeds the maximum file size' do
           expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "File \"README\" is larger than the allowed size of 1 MB")
         end

spec/models/ee/project_spec.rb

@@ -11,6 +11,22 @@ describe Project, models: true do
     it { is_expected.to delegate_method(:shared_runners_minutes_used?).to(:namespace) }
   end
 
+  describe '#push_rule' do
+    let(:project) { create(:project, push_rule: create(:push_rule)) }
+
+    subject(:push_rule) { project.push_rule(true) }
+
+    it { is_expected.not_to be_nil }
+
+    context 'push rules unlicensed' do
+      before do
+        stub_licensed_features(push_rules: false)
+      end
+
+      it { is_expected.to be_nil }
+    end
+  end
+
   describe '#feature_available?' do
     let(:namespace) { build_stubbed(:namespace) }
     let(:project) { build_stubbed(:project, namespace: namespace) }

spec/services/ee/projects/create_service_spec.rb

@@ -34,13 +34,26 @@ describe Projects::CreateService, '#execute', services: true do
   end
 
   context 'git hook sample' do
+    let!(:sample) { create(:push_rule_sample) }
+
+    subject(:push_rule) { create_project(user, opts).push_rule }
+
     it 'creates git hook from sample' do
-      push_rule_sample = create(:push_rule_sample)
+      is_expected.to have_attributes(
+        force_push_regex: sample.force_push_regex,
+        deny_delete_tag: sample.deny_delete_tag,
+        delete_branch_regex: sample.delete_branch_regex,
+        commit_message_regex: sample.commit_message_regex
+      )
+    end
 
-      push_rule = create_project(user, opts).push_rule
+    context 'push rules unlicensed' do
+      before do
+        stub_licensed_features(push_rules: false)
+      end
 
-      [:force_push_regex, :deny_delete_tag, :delete_branch_regex, :commit_message_regex].each do |attr_name|
-        expect(push_rule.send(attr_name)).to eq push_rule_sample.send(attr_name)
+      it 'ignores the push rule sample' do
+        is_expected.to be_nil
       end
     end
   end

spec/services/merge_requests/merge_service_spec.rb

@@ -242,6 +242,16 @@ describe MergeRequests::MergeService, services: true do
   end
 
   describe '#hooks_validation_pass?' do
+    shared_examples 'hook validations are skipped when push rules unlicensed' do
+      subject { service.hooks_validation_pass?(merge_request) }
+
+      before do
+        stub_licensed_features(push_rules: false)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+
     let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') }
 
     it 'returns true when valid' do
@@ -253,6 +263,8 @@ describe MergeRequests::MergeService, services: true do
         allow(project).to receive(:push_rule) { build(:push_rule, commit_message_regex: 'unmatched pattern .*') }
       end
 
+      it_behaves_like 'hook validations are skipped when push rules unlicensed'
+
       it 'returns false and saves error when invalid' do
         expect(service.hooks_validation_pass?(merge_request)).to be_falsey
         expect(merge_request.merge_error).not_to be_empty
@@ -264,6 +276,8 @@ describe MergeRequests::MergeService, services: true do
         allow(project).to receive(:push_rule) { build(:push_rule, author_email_regex: '.*@unmatchedemaildomain.com') }
       end
 
+      it_behaves_like 'hook validations are skipped when push rules unlicensed'
+
       it 'returns false and saves error when invalid' do
         expect(service.hooks_validation_pass?(merge_request)).to be_falsey
         expect(merge_request.merge_error).not_to be_empty