Merge branch '28084-project-webhook-limits' into 'master'

Configure maximum number of webhooks per project

See merge request gitlab-org/gitlab!20730

app/models/project.rb

@@ -334,7 +334,7 @@ class Project < ApplicationRecord
   delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_role, to: :team
   delegate :add_master, to: :team # @deprecated
   delegate :group_runners_enabled, :group_runners_enabled=, :group_runners_enabled?, to: :ci_cd_settings
-  delegate :root_ancestor, to: :namespace, allow_nil: true
+  delegate :root_ancestor, :actual_limits, to: :namespace, allow_nil: true
   delegate :last_pipeline, to: :commit, allow_nil: true
   delegate :external_dashboard_url, to: :metrics_setting, allow_nil: true, prefix: true
   delegate :default_git_depth, :default_git_depth=, to: :ci_cd_settings, prefix: :ci

db/migrate/20191216094119_add_id_to_plan_limits.rb

+# frozen_string_literal: true
+
+class AddIdToPlanLimits < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def up
+    add_column(:plan_limits, :id, :primary_key) unless column_exists?(:plan_limits, :id)
+  end
+
+  def down
+    remove_column(:plan_limits, :id)
+  end
+end

db/migrate/20191216183531_add_project_hooks_to_plan_limits.rb

+# frozen_string_literal: true
+
+class AddProjectHooksToPlanLimits < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column(:plan_limits, :project_hooks, :integer, default: 0, null: false)
+  end
+end

db/migrate/20191216183532_insert_project_hooks_plan_limits.rb

+# frozen_string_literal: true
+
+class InsertProjectHooksPlanLimits < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    return unless Gitlab.com?
+
+    create_or_update_plan_limit('project_hooks', 'free', 10)
+    create_or_update_plan_limit('project_hooks', 'bronze', 20)
+    create_or_update_plan_limit('project_hooks', 'silver', 30)
+    create_or_update_plan_limit('project_hooks', 'gold', 100)
+  end
+
+  def down
+    return unless Gitlab.com?
+
+    create_or_update_plan_limit('project_hooks', 'free', 0)
+    create_or_update_plan_limit('project_hooks', 'bronze', 0)
+    create_or_update_plan_limit('project_hooks', 'silver', 0)
+    create_or_update_plan_limit('project_hooks', 'gold', 0)
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_12_14_175727) do
+ActiveRecord::Schema.define(version: 2019_12_16_183532) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -2987,6 +2987,7 @@ ActiveRecord::Schema.define(version: 2019_12_14_175727) do
     t.integer "ci_active_pipelines", default: 0, null: false
     t.integer "ci_pipeline_size", default: 0, null: false
     t.integer "ci_active_jobs", default: 0, null: false
+    t.integer "project_hooks", default: 0, null: false
     t.index ["plan_id"], name: "index_plan_limits_on_plan_id", unique: true
   end
 

doc/user/project/integrations/webhooks.md

@@ -47,6 +47,20 @@ and **per project and per group** for **GitLab Enterprise Edition**.
 Navigate to the webhooks page by going to your project's
 **Settings ➔ Integrations**.
 
+## Maximum number of webhooks (per tier)
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/20730) in GitLab 12.6.
+
+A maximum number of project webhooks applies to each [GitLab.com
+tier](https://about.gitlab.com/pricing/), as shown in the following table:
+
+| Tier     | Number of webhooks per project |
+|----------|--------------------------------|
+| Free     | 10                             |
+| Bronze   | 20                             |
+| Silver   | 30                             |
+| Gold     | 100                            |
+
 ## Use-cases
 
 - You can set up a webhook in GitLab to send a notification to

ee/app/models/concerns/ee/limitable.rb

+# frozen_string_literal: true
+
+module EE
+  module Limitable
+    extend ActiveSupport::Concern
+
+    included do
+      validate :validate_plan_limit_not_exceeded, on: :create
+    end
+
+    private
+
+    def validate_plan_limit_not_exceeded
+      return unless project
+
+      limit_name = self.class.name.demodulize.tableize
+      relation = self.class.where(project: project)
+
+      if project.actual_limits.exceeded?(limit_name, relation)
+        errors.add(:base, _("Maximum number of %{name} (%{count}) exceeded") %
+          { name: limit_name.humanize(capitalize: false), count: project.actual_limits.public_send(limit_name) }) # rubocop:disable GitlabSecurity/PublicSend
+      end
+    end
+  end
+end

ee/app/models/ee/project_hook.rb

@@ -6,6 +6,7 @@ module EE
 
     prepended do
       include CustomModelNaming
+      include Limitable
 
       self.singular_route_key = :hook
     end

ee/app/models/plan_limits.rb

 # frozen_string_literal: true
 
 class PlanLimits < ApplicationRecord
-  self.primary_key = :plan_id
-
   belongs_to :plan
+
+  def exceeded?(limit_name, relation)
+    return false unless enabled?(limit_name)
+
+    # relation.count >= limit value is slower than checking
+    # if a record exists at the limit value - 1 position.
+    relation.limit(1).offset(read_attribute(limit_name) - 1).exists?
+  end
+
+  private
+
+  def enabled?(limit_name)
+    read_attribute(limit_name) > 0
+  end
 end

ee/changelogs/unreleased/28084-project-webhook-limits.yml

+---
+title: Add project webhooks limits on GitLab.com
+merge_request: 20730
+author:
+type: other

ee/spec/models/ee/project_hook_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProjectHook do
+  subject(:project_hook) { build(:project_hook, project: project) }
+
+  let(:gold_plan) { create(:gold_plan) }
+  let(:plan_limits) { create(:plan_limits, plan: gold_plan) }
+  let(:namespace) { create(:namespace) }
+  let(:project) { create(:project, namespace: namespace) }
+  let!(:subscription) { create(:gitlab_subscription, namespace: namespace, hosted_plan: gold_plan) }
+
+  context 'without plan limits configured' do
+    it 'can create new project hooks' do
+      expect { project_hook.save }.to change { described_class.count }
+    end
+  end
+
+  context 'with plan limits configured' do
+    before do
+      plan_limits.update(project_hooks: 1)
+    end
+
+    it 'can create new project hooks' do
+      expect { project_hook.save }.to change { described_class.count }
+    end
+
+    it 'cannot create new project hooks exceding the plan limits' do
+      create(:project_hook, project: project)
+
+      expect { project_hook.save }.not_to change { described_class.count }
+      expect(project_hook.errors[:base]).to contain_exactly('Maximum number of project hooks (1) exceeded')
+    end
+  end
+end

lib/gitlab/database/migration_helpers.rb

@@ -1052,6 +1052,15 @@ into similar problems in the future (e.g. when new tables are created).
         connection.select_value(index_sql).to_i > 0
       end
 
+      def create_or_update_plan_limit(limit_name, plan_name, limit_value)
+        execute <<~SQL
+          INSERT INTO plan_limits (plan_id, #{quote_column_name(limit_name)})
+          VALUES
+            ((SELECT id FROM plans WHERE name = #{quote(plan_name)} LIMIT 1), #{quote(limit_value)})
+          ON CONFLICT (plan_id) DO UPDATE SET #{quote_column_name(limit_name)} = EXCLUDED.#{quote_column_name(limit_name)};
+        SQL
+      end
+
       private
 
       def tables_match?(target_table, foreign_key_table)

locale/gitlab.pot

@@ -10985,6 +10985,9 @@ msgstr ""
 msgid "Maximum lifetime allowable for Personal Access Tokens is active, your expire date must be set before %{maximum_allowable_date}."
 msgstr ""
 
+msgid "Maximum number of %{name} (%{count}) exceeded"
+msgstr ""
+
 msgid "Maximum number of comments exceeded"
 msgstr ""
 

rubocop/cop/migration/add_column.rb

@@ -8,7 +8,10 @@ module RuboCop
       class AddColumn < RuboCop::Cop::Cop
         include MigrationHelpers
 
-        WHITELISTED_TABLES = [:application_settings].freeze
+        WHITELISTED_TABLES = %i[
+          application_settings
+          plan_limits
+        ].freeze
 
         MSG = '`add_column` with a default value requires downtime, ' \
           'use `add_column_with_default` instead'.freeze

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -1440,4 +1440,17 @@ describe Gitlab::Database::MigrationHelpers do
       end
     end
   end
+
+  describe '#create_or_update_plan_limit' do
+    it 'creates or updates plan limits' do
+      expect(model).to receive(:execute).with <<~SQL
+        INSERT INTO plan_limits (plan_id, "project_hooks")
+        VALUES
+          ((SELECT id FROM plans WHERE name = 'free' LIMIT 1), '10')
+        ON CONFLICT (plan_id) DO UPDATE SET "project_hooks" = EXCLUDED."project_hooks";
+      SQL
+
+      model.create_or_update_plan_limit('project_hooks', 'free', 10)
+    end
+  end
 end

spec/migrations/insert_project_hooks_plan_limits_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20191216183532_insert_project_hooks_plan_limits.rb')
+
+describe InsertProjectHooksPlanLimits, :migration do
+  let(:migration) { described_class.new }
+  let(:plans) { table(:plans) }
+  let(:plan_limits) { table(:plan_limits) }
+
+  before do
+    plans.create(id: 34, name: 'free')
+    plans.create(id: 2, name: 'bronze')
+    plans.create(id: 3, name: 'silver')
+    plans.create(id: 4, name: 'gold')
+    plan_limits.create(plan_id: 34, ci_active_jobs: 5)
+  end
+
+  context 'when on Gitlab.com' do
+    before do
+      expect(Gitlab).to receive(:com?).at_most(:twice).and_return(true)
+    end
+
+    describe '#up' do
+      it 'updates the project_hooks plan limits' do
+        migration.up
+
+        expect(plan_limits.pluck(:plan_id, :project_hooks, :ci_active_jobs))
+          .to match_array([[34, 10, 5], [2, 20, 0], [3, 30, 0], [4, 100, 0]])
+      end
+    end
+
+    describe '#down' do
+      it 'updates the project_hooks plan limits to 0' do
+        migration.up
+        migration.down
+
+        expect(plan_limits.pluck(:plan_id, :project_hooks, :ci_active_jobs))
+          .to match_array([[34, 0, 5], [2, 0, 0], [3, 0, 0], [4, 0, 0]])
+      end
+    end
+  end
+
+  context 'when on self-hosted' do
+    before do
+      expect(Gitlab).to receive(:com?).and_return(false)
+    end
+
+    describe '#up' do
+      it 'does not update the plan limits' do
+        migration.up
+
+        expect(plan_limits.pluck(:plan_id, :project_hooks, :ci_active_jobs))
+          .to match_array([[34, 0, 5]])
+      end
+    end
+
+    describe '#down' do
+      it 'does not update the plan limits' do
+        migration.down
+
+        expect(plan_limits.pluck(:plan_id, :project_hooks, :ci_active_jobs))
+          .to match_array([[34, 0, 5]])
+      end
+    end
+  end
+end