Add latest changes from gitlab-org/gitlab@master

8 files changed, 216 insertions, 67 deletions

diff --git a/lib/api/entities/hook.rb b/lib/api/entities/hook.rb
index d176e76b321..95924321221 100644
--- a/lib/api/entities/hook.rb
+++ b/lib/api/entities/hook.rb
@@ -8,6 +8,11 @@ module API
 
       expose :alert_status
       expose :disabled_until
+      expose :url_variables
+
+      def url_variables
+        object.url_variables.keys.map { { key: _1 } }
+      end
     end
   end
 end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index fc1037131d8..c73a5482cac 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -476,9 +476,9 @@ module API
       render_api_error!('202 Accepted', 202)
     end
 
-    def render_validation_error!(model)
+    def render_validation_error!(model, status = 400)
       if model.errors.any?
-        render_api_error!(model_error_messages(model) || '400 Bad Request', 400)
+        render_api_error!(model_error_messages(model) || '400 Bad Request', status)
       end
     end
 
diff --git a/lib/api/helpers/web_hooks_helpers.rb b/lib/api/helpers/web_hooks_helpers.rb
new file mode 100644
index 00000000000..a71e56af4c3
--- /dev/null
+++ b/lib/api/helpers/web_hooks_helpers.rb
@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+module API
+  module Helpers
+    module WebHooksHelpers
+      extend Grape::API::Helpers
+
+      params :requires_url do
+        requires :url, type: String, desc: "The URL to send the request to"
+      end
+
+      params :optional_url do
+        optional :url, type: String, desc: "The URL to send the request to"
+      end
+
+      params :url_variables do
+        optional :url_variables, type: Array, desc: 'URL variables for interpolation' do
+          requires :key, type: String, desc: 'Name of the variable'
+          requires :value, type: String, desc: 'Value of the variable'
+        end
+      end
+
+      def find_hook
+        hook_scope.find(params.delete(:hook_id))
+      end
+
+      def create_hook_params
+        hook_params = declared_params(include_missing: false)
+        url_variables = hook_params.delete(:url_variables)
+
+        if url_variables.present?
+          hook_params[:url_variables] = url_variables.to_h { [_1[:key], _1[:value]] }
+        end
+
+        hook_params
+      end
+
+      def update_hook(entity:)
+        hook = find_hook
+        update_params = update_hook_params(hook)
+
+        hook.assign_attributes(update_params)
+
+        save_hook(hook, entity)
+      end
+
+      def update_hook_params(hook)
+        update_params = declared_params(include_missing: false)
+        url_variables = update_params.delete(:url_variables) || []
+        url_variables = url_variables.to_h { [_1[:key], _1[:value]] }
+        update_params[:url_variables] = hook.url_variables.merge(url_variables) if url_variables.present?
+
+        error!('No parameters provided', :bad_request) if update_params.empty?
+
+        update_params
+      end
+
+      def save_hook(hook, entity)
+        if hook.save
+          present hook, with: entity
+        else
+          error!("Invalid url given", 422) if hook.errors[:url].present?
+          error!("Invalid branch filter given", 422) if hook.errors[:push_events_branch_filter].present?
+
+          render_validation_error!(hook, 422)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/api/hooks/test.rb b/lib/api/hooks/test.rb
new file mode 100644
index 00000000000..4871955c6e0
--- /dev/null
+++ b/lib/api/hooks/test.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module API
+  module Hooks
+    # It is important that this re-usable module is not a Grape Instance,
+    # since it will be re-mounted.
+    # rubocop: disable API/Base
+    class Test < ::Grape::API
+      params do
+        requires :hook_id, type: Integer, desc: 'The ID of the hook'
+      end
+      post ":hook_id" do
+        hook = find_hook
+        data = configuration[:data].dup
+        hook.execute(data, configuration[:kind])
+        data
+      end
+    end
+    # rubocop: enable API/Base
+  end
+end
diff --git a/lib/api/hooks/url_variables.rb b/lib/api/hooks/url_variables.rb
new file mode 100644
index 00000000000..708b78134e5
--- /dev/null
+++ b/lib/api/hooks/url_variables.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+module API
+  module Hooks
+    # It is important that this re-usable module is not a Grape Instance,
+    # since it will be re-mounted.
+    # rubocop: disable API/Base
+    class UrlVariables < ::Grape::API
+      params do
+        requires :hook_id, type: Integer, desc: 'The ID of the hook'
+        requires :key, type: String, desc: 'The key of the variable'
+      end
+      namespace ':hook_id/url_variables' do
+        desc 'Set a url variable'
+        params do
+          requires :value, type: String, desc: 'The value of the variable'
+        end
+        put ":key" do
+          hook = find_hook
+          key = params.delete(:key)
+          value = params.delete(:value)
+          vars = hook.url_variables.merge(key => value)
+
+          error!('Illegal key or value', 422) unless hook.update(url_variables: vars)
+
+          status :no_content
+        end
+
+        desc 'Un-Set a url variable'
+        delete ":key" do
+          hook = find_hook
+          key = params.delete(:key)
+          not_found!('URL variable') unless hook.url_variables.key?(key)
+
+          vars = hook.url_variables.reject { _1 == key }
+
+          error!('Could not unset variable', 422) unless hook.update(url_variables: vars)
+
+          status :no_content
+        end
+      end
+    end
+    # rubocop: enable API/Base
+  end
+end
diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb
index 3edd38a0108..eb52e8d2c0f 100644
--- a/lib/api/internal/base.rb
+++ b/lib/api/internal/base.rb
@@ -164,13 +164,15 @@ module API
           check_allowed(params)
         end
 
-        post '/error_tracking_allowed', feature_category: :error_tracking do
+        post '/error_tracking/allowed', feature_category: :error_tracking do
           public_key = params[:public_key]
           project_id = params[:project_id]
 
           unprocessable_entity! if public_key.blank? || project_id.blank?
 
-          enabled = ::ErrorTracking::ClientKey.enabled_key_for(project_id, public_key).exists?
+          project = Project.find(project_id)
+          enabled = Feature.enabled?(:use_click_house_database_for_error_tracking, project) &&
+            ::ErrorTracking::ClientKey.enabled_key_for(project_id, public_key).exists?
 
           status 200
           { enabled: enabled }
diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb
index 431ba199131..466e80d68c8 100644
--- a/lib/api/project_hooks.rb
+++ b/lib/api/project_hooks.rb
@@ -9,16 +9,21 @@ module API
 
     feature_category :integrations
 
+    helpers ::API::Helpers::WebHooksHelpers
+
     helpers do
-      params :project_hook_properties do
-        requires :url, type: String, desc: "The URL to send the request to"
+      def hook_scope
+        user_project.hooks
+      end
+
+      params :common_hook_parameters do
         optional :push_events, type: Boolean, desc: "Trigger hook on push events"
         optional :issues_events, type: Boolean, desc: "Trigger hook on issues events"
         optional :confidential_issues_events, type: Boolean, desc: "Trigger hook on confidential issues events"
         optional :merge_requests_events, type: Boolean, desc: "Trigger hook on merge request events"
         optional :tag_push_events, type: Boolean, desc: "Trigger hook on tag push events"
-        optional :note_events, type: Boolean, desc: "Trigger hook on note(comment) events"
-        optional :confidential_note_events, type: Boolean, desc: "Trigger hook on confidential note(comment) events"
+        optional :note_events, type: Boolean, desc: "Trigger hook on note (comment) events"
+        optional :confidential_note_events, type: Boolean, desc: "Trigger hook on confidential note (comment) events"
         optional :job_events, type: Boolean, desc: "Trigger hook on job events"
         optional :pipeline_events, type: Boolean, desc: "Trigger hook on pipeline events"
         optional :wiki_page_events, type: Boolean, desc: "Trigger hook on wiki events"
@@ -27,6 +32,7 @@ module API
         optional :enable_ssl_verification, type: Boolean, desc: "Do SSL verification when triggering the hook"
         optional :token, type: String, desc: "Secret token to validate received payloads; this will not be returned in the response"
         optional :push_events_branch_filter, type: String, desc: "Trigger hook on specified branch only"
+        use :url_variables
       end
     end
 
@@ -34,6 +40,10 @@ module API
       requires :id, type: String, desc: 'The ID of a project'
     end
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+      namespace ':id/hooks' do
+        mount ::API::Hooks::UrlVariables
+      end
+
       desc 'Get project hooks' do
         success Entities::ProjectHook
       end
@@ -59,43 +69,26 @@ module API
         success Entities::ProjectHook
       end
       params do
-        use :project_hook_properties
+        use :requires_url
+        use :common_hook_parameters
       end
       post ":id/hooks" do
-        hook_params = declared_params(include_missing: false)
-
+        hook_params = create_hook_params
         hook = user_project.hooks.new(hook_params)
 
-        if hook.save
-          present hook, with: Entities::ProjectHook
-        else
-          error!("Invalid url given", 422) if hook.errors[:url].present?
-          error!("Invalid branch filter given", 422) if hook.errors[:push_events_branch_filter].present?
-
-          not_found!("Project hook #{hook.errors.messages}")
-        end
+        save_hook(hook, Entities::ProjectHook)
       end
 
-      desc 'Update an existing project hook' do
+      desc 'Update an existing hook' do
         success Entities::ProjectHook
       end
       params do
         requires :hook_id, type: Integer, desc: "The ID of the hook to update"
-        use :project_hook_properties
+        use :optional_url
+        use :common_hook_parameters
       end
       put ":id/hooks/:hook_id" do
-        hook = user_project.hooks.find(params.delete(:hook_id))
-
-        update_params = declared_params(include_missing: false)
-
-        if hook.update(update_params)
-          present hook, with: Entities::ProjectHook
-        else
-          error!("Invalid url given", 422) if hook.errors[:url].present?
-          error!("Invalid branch filter given", 422) if hook.errors[:push_events_branch_filter].present?
-
-          not_found!("Project hook #{hook.errors.messages}")
-        end
+        update_hook(entity: Entities::ProjectHook)
       end
 
       desc 'Deletes project hook' do
@@ -105,7 +98,7 @@ module API
         requires :hook_id, type: Integer, desc: 'The ID of the hook to delete'
       end
       delete ":id/hooks/:hook_id" do
-        hook = user_project.hooks.find(params.delete(:hook_id))
+        hook = find_hook
 
         destroy_conditionally!(hook) do
           WebHooks::DestroyService.new(current_user).execute(hook)
diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb
index 7c91fbd36d9..804cedfefe9 100644
--- a/lib/api/system_hooks.rb
+++ b/lib/api/system_hooks.rb
@@ -11,7 +11,27 @@ module API
       authenticated_as_admin!
     end
 
+    helpers ::API::Helpers::WebHooksHelpers
+
+    helpers do
+      def hook_scope
+        SystemHook
+      end
+
+      params :hook_parameters do
+        optional :token, type: String, desc: 'The token used to validate payloads'
+        optional :push_events, type: Boolean, desc: "Trigger hook on push events"
+        optional :tag_push_events, type: Boolean, desc: "Trigger hook on tag push events"
+        optional :merge_requests_events, type: Boolean, desc: "Trigger hook on tag push events"
+        optional :repository_update_events, type: Boolean, desc: "Trigger hook on repository update events"
+        optional :enable_ssl_verification, type: Boolean, desc: "Do SSL verification when triggering the hook"
+        use :url_variables
+      end
+    end
+
     resource :hooks do
+      mount ::API::Hooks::UrlVariables
+
       desc 'Get the list of system hooks' do
         success Entities::Hook
       end
@@ -26,70 +46,63 @@ module API
         success Entities::Hook
       end
       params do
-        requires :id, type: Integer, desc: 'The ID of the system hook'
+        requires :hook_id, type: Integer, desc: 'The ID of the system hook'
       end
-      get ":id" do
-        hook = SystemHook.find(params[:id])
-
-        present hook, with: Entities::Hook
+      get ":hook_id" do
+        present find_hook, with: Entities::Hook
       end
 
       desc 'Create a new system hook' do
         success Entities::Hook
       end
       params do
-        requires :url, type: String, desc: "The URL to send the request to"
-        optional :token, type: String, desc: 'The token used to validate payloads'
-        optional :push_events, type: Boolean, desc: "Trigger hook on push events"
-        optional :tag_push_events, type: Boolean, desc: "Trigger hook on tag push events"
-        optional :merge_requests_events, type: Boolean, desc: "Trigger hook on tag push events"
-        optional :repository_update_events, type: Boolean, desc: "Trigger hook on repository update events"
-        optional :enable_ssl_verification, type: Boolean, desc: "Do SSL verification when triggering the hook"
+        use :requires_url
+        use :hook_parameters
       end
       post do
-        hook = SystemHook.new(declared_params(include_missing: false))
+        hook_params = create_hook_params
+        hook = SystemHook.new(hook_params)
 
-        if hook.save
-          present hook, with: Entities::Hook
-        else
-          render_validation_error!(hook)
-        end
+        save_hook(hook, Entities::Hook)
       end
 
-      desc 'Test a hook'
+      desc 'Update an existing system hook' do
+        success Entities::Hook
+      end
       params do
-        requires :id, type: Integer, desc: 'The ID of the system hook'
+        requires :hook_id, type: Integer, desc: "The ID of the hook to update"
+        use :optional_url
+        use :hook_parameters
       end
-      post ":id" do
-        hook = SystemHook.find(params[:id])
-        data = {
+      put ":hook_id" do
+        update_hook(entity: Entities::Hook)
+      end
+
+      mount ::API::Hooks::Test, with: {
+        data: {
           event_name: "project_create",
           name: "Ruby",
           path: "ruby",
           project_id: 1,
           owner_name: "Someone",
           owner_email: "example@gitlabhq.com"
-        }
-        hook.execute(data, 'system_hooks')
-        data
-      end
+        },
+        kind: 'system_hooks'
+      }
 
       desc 'Delete a hook' do
         success Entities::Hook
       end
       params do
-        requires :id, type: Integer, desc: 'The ID of the system hook'
+        requires :hook_id, type: Integer, desc: 'The ID of the system hook'
       end
-      # rubocop: disable CodeReuse/ActiveRecord
-      delete ":id" do
-        hook = SystemHook.find_by(id: params[:id])
-        not_found!('System hook') unless hook
+      delete ":hook_id" do
+        hook = find_hook
 
         destroy_conditionally!(hook) do
           WebHooks::DestroyService.new(current_user).execute(hook)
         end
       end
-      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end