Merge branch 'ee-parse-unknown-attrs-for-generic-alert' into 'master'

Parse extra payload parameters for Generic Alert

See merge request gitlab-org/gitlab!16441

ee/app/services/projects/alerting/notify_service.rb

@@ -17,6 +17,8 @@ module Projects
         process_incident_issues
 
         ServiceResponse.success
+      rescue Gitlab::Alerting::NotificationPayloadParser::BadPayloadError
+        bad_request
       end
 
       private
@@ -46,6 +48,10 @@ module Projects
         token == DEV_TOKEN
       end
 
+      def bad_request
+        ServiceResponse.error(message: 'Bad Request', http_status: 400)
+      end
+
       def unauthorized
         ServiceResponse.error(message: 'Unauthorized', http_status: 401)
       end

ee/lib/gitlab/alerting/notification_payload_parser.rb

@@ -3,6 +3,8 @@
 module Gitlab
   module Alerting
     class NotificationPayloadParser
+      BadPayloadError = Class.new(StandardError)
+
       DEFAULT_TITLE = 'New: Incident'
 
       def initialize(payload)
@@ -29,17 +31,24 @@ module Gitlab
       end
 
       def annotations
+        primary_params
+          .reverse_merge(flatten_secondary_params)
+          .transform_values(&:presence)
+          .compact
+      end
+
+      def primary_params
         {
           'title' => title,
-          'description' => payload[:description].presence,
-          'monitoring_tool' => payload[:monitoring_tool].presence,
-          'service' => payload[:service].presence,
-          'hosts' => hosts
-        }.compact
+          'description' => payload[:description],
+          'monitoring_tool' => payload[:monitoring_tool],
+          'service' => payload[:service],
+          'hosts' => hosts.presence
+        }
       end
 
       def hosts
-        Array(payload[:hosts]).reject(&:blank?).presence
+        Array(payload[:hosts]).reject(&:blank?)
       end
 
       def current_time
@@ -51,6 +60,16 @@ module Gitlab
       rescue ArgumentError
         current_time
       end
+
+      def secondary_params
+        payload.except(:start_time)
+      end
+
+      def flatten_secondary_params
+        Gitlab::Utils::SafeInlineHash.merge_keys!(secondary_params)
+      rescue ArgumentError
+        raise BadPayloadError, 'The payload is too big'
+      end
     end
   end
 end

ee/spec/lib/gitlab/alerting/notification_payload_parser_spec.rb

@@ -93,5 +93,42 @@ describe Gitlab::Alerting::NotificationPayloadParser do
         )
       end
     end
+
+    context 'when payload has secondary params' do
+      let(:payload) do
+        {
+          'description' => 'Description',
+          'additional' => {
+            'params' => {
+              '1' => 'Some value 1',
+              '2' => 'Some value 2',
+              'blank' => ''
+            }
+          }
+        }
+      end
+
+      it 'adds secondary params to annotations' do
+        is_expected.to eq(
+          'annotations' => {
+            'title' => 'New: Incident',
+            'description' => 'Description',
+            'additional.params.1' => 'Some value 1',
+            'additional.params.2' => 'Some value 2'
+          },
+          'startsAt' => starts_at.rfc3339
+        )
+      end
+    end
+
+    context 'when secondary params hash is too big' do
+      before do
+        allow(Gitlab::Utils::SafeInlineHash).to receive(:merge_keys!).and_raise(ArgumentError)
+      end
+
+      it 'catches and re-raises an error' do
+        expect { subject }.to raise_error Gitlab::Alerting::NotificationPayloadParser::BadPayloadError, 'The payload is too big'
+      end
+    end
   end
 end

ee/spec/services/projects/alerting/notify_service_spec.rb

@@ -60,9 +60,21 @@ describe Projects::Alerting::NotifyService do
         end
 
         context 'with valid token' do
+          context 'with a valid payload' do
             it_behaves_like 'processes incident issues', 1
           end
 
+          context 'with an invalid payload' do
+            before do
+              allow(Gitlab::Alerting::NotificationPayloadParser)
+                .to receive(:call)
+                .and_raise(Gitlab::Alerting::NotificationPayloadParser::BadPayloadError)
+            end
+
+            it_behaves_like 'does not process incident issues', http_status: 400
+          end
+        end
+
         context 'with invalid token' do
           let(:token) { 'invalid-token' }
 

lib/gitlab/utils/inline_hash.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Utils
+    module InlineHash
+      extend self
+
+      # Transforms a Hash into an inline Hash by merging its nested keys.
+      #
+      # Input
+      #
+      #  {
+      #    'root_param' => 'Root',
+      #    nested_param: {
+      #      key: 'Value'
+      #    },
+      #    'very' => {
+      #      'deep' => {
+      #        'nested' => {
+      #          'param' => 'Deep nested value'
+      #        }
+      #      }
+      #    }
+      #  }
+      #
+      #
+      # Result
+      #
+      #  {
+      #    'root_param' => 'Root',
+      #    'nested_param.key' => 'Value',
+      #    'very.deep.nested.param' => 'Deep nested value'
+      #  }
+      #
+      def merge_keys(hash, prefix: nil, connector: '.')
+        result = {}
+        base_prefix = prefix ? "#{prefix}#{connector}" : ''
+        pairs = hash.map { |key, value| ["#{base_prefix}#{key}", value] }
+
+        until pairs.empty?
+          key, value = pairs.shift
+
+          if value.is_a?(Hash)
+            value.each { |k, v| pairs.unshift ["#{key}#{connector}#{k}", v] }
+          else
+            result[key] = value
+          end
+        end
+
+        result
+      end
+    end
+  end
+end

lib/gitlab/utils/safe_inline_hash.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Utils
+    class SafeInlineHash
+      # Validates the hash size using `Gitlab::Utils::DeepSize` before merging keys using `Gitlab::Utils::InlineHash`
+      def initialize(hash, prefix: nil, connector: '.')
+        @hash = hash
+      end
+
+      def self.merge_keys!(hash, prefix: nil, connector: '.')
+        new(hash).merge_keys!(prefix: prefix, connector: connector)
+      end
+
+      def merge_keys!(prefix:, connector:)
+        raise ArgumentError, 'The Hash is too big' unless valid?
+
+        Gitlab::Utils::InlineHash.merge_keys(hash, prefix: prefix, connector: connector)
+      end
+
+      private
+
+      attr_reader :hash
+
+      def valid?
+        Gitlab::Utils::DeepSize.new(hash).valid?
+      end
+    end
+  end
+end

spec/lib/gitlab/utils/inline_hash_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+describe Gitlab::Utils::InlineHash do
+  describe '.merge_keys' do
+    subject { described_class.merge_keys(source) }
+
+    let(:source) do
+      {
+        nested_param: {
+          key: 'Value'
+        },
+        'root_param' => 'Root',
+        'very' => {
+          'deep' => {
+            'nested' => {
+              'param' => 'Deep nested value'
+            }
+          }
+        }
+      }
+    end
+
+    it 'transforms a nested hash into a one-level hash' do
+      is_expected.to eq(
+        'nested_param.key' => 'Value',
+        'root_param' => 'Root',
+        'very.deep.nested.param' => 'Deep nested value'
+      )
+    end
+
+    it 'retains key insertion order' do
+      expect(subject.keys)
+        .to eq(%w(nested_param.key root_param very.deep.nested.param))
+    end
+
+    context 'with a custom connector' do
+      subject { described_class.merge_keys(source, connector: '::') }
+
+      it 'uses the connector to merge keys' do
+        is_expected.to eq(
+          'nested_param::key' => 'Value',
+          'root_param' => 'Root',
+          'very::deep::nested::param' => 'Deep nested value'
+        )
+      end
+    end
+
+    context 'with a starter prefix' do
+      subject { described_class.merge_keys(source, prefix: 'options') }
+
+      it 'prefixes all the keys' do
+        is_expected.to eq(
+          'options.nested_param.key' => 'Value',
+          'options.root_param' => 'Root',
+          'options.very.deep.nested.param' => 'Deep nested value'
+        )
+      end
+    end
+  end
+end

spec/lib/gitlab/utils/safe_inline_hash_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+describe Gitlab::Utils::SafeInlineHash do
+  context '.merge_keys!' do
+    let(:source) { { 'foo' => { 'bar' => 'baz' } } }
+    let(:validator) { instance_double(Gitlab::Utils::DeepSize, valid?: valid) }
+
+    subject { described_class.merge_keys!(source, prefix: 'safe', connector: '::') }
+
+    before do
+      allow(Gitlab::Utils::DeepSize)
+        .to receive(:new)
+        .with(source)
+        .and_return(validator)
+    end
+
+    context 'when hash is too big' do
+      let(:valid) { false }
+
+      it 'raises an exception' do
+        expect { subject }.to raise_error ArgumentError, 'The Hash is too big'
+      end
+    end
+
+    context 'when hash has an acceptaable size' do
+      let(:valid) { true }
+
+      it 'returns a result of InlineHash' do
+        is_expected.to eq('safe::foo::bar' => 'baz')
+      end
+    end
+  end
+end