Merge branch 'sy-support-arrays-in-custom-mappings' into 'master'

Add support for arrays to HTTP integration custom mappings

See merge request gitlab-org/gitlab!58610

doc/api/graphql/reference/index.md

@@ -631,7 +631,7 @@ Parsed field from an alert used for custom mappings.
 | Field | Type | Description |
 | ----- | ---- | ----------- |
 | `label` | [`String`](#string) | Human-readable label of the payload path. |
-| `path` | [`[String!]`](#string) | Path to value inside payload JSON. |
+| `path` | [`[PayloadAlertFieldPathSegment!]`](#payloadalertfieldpathsegment) | Path to value inside payload JSON. |
 | `type` | [`AlertManagementPayloadAlertFieldType`](#alertmanagementpayloadalertfieldtype) | Type of the parsed value. |
 
 ### `AlertManagementPayloadAlertMappingField`
@@ -642,7 +642,7 @@ Parsed field (with its name) from an alert used for custom mappings.
 | ----- | ---- | ----------- |
 | `fieldName` | [`AlertManagementPayloadAlertFieldName`](#alertmanagementpayloadalertfieldname) | A GitLab alert field name. |
 | `label` | [`String`](#string) | Human-readable label of the payload path. |
-| `path` | [`[String!]`](#string) | Path to value inside payload JSON. |
+| `path` | [`[PayloadAlertFieldPathSegment!]`](#payloadalertfieldpathsegment) | Path to value inside payload JSON. |
 | `type` | [`AlertManagementPayloadAlertFieldType`](#alertmanagementpayloadalertfieldtype) | Type of the parsed value. |
 
 ### `AlertManagementPrometheusIntegration`
@@ -8998,6 +8998,10 @@ A `PackagesPackageID` is a global ID. It is encoded as a string.
 
 An example `PackagesPackageID` is: `"gid://gitlab/Packages::Package/1"`.
 
+### `PayloadAlertFieldPathSegment`
+
+String or integer.
+
 ### `ProjectID`
 
 A `ProjectID` is a global ID. It is encoded as a string.

ee/app/graphql/types/alert_management/payload_alert_field_input_type.rb

@@ -12,7 +12,7 @@ module Types
                 description: 'A GitLab alert field name.'
 
       argument :path,
-               [GraphQL::STRING_TYPE],
+               [::Types::AlertManagement::PayloadAlertFieldPathSegmentType],
                required: true,
                description: 'Path to value inside payload JSON.'
 

ee/app/graphql/types/alert_management/payload_alert_field_path_segment_type.rb

+# frozen_string_literal: true
+
+module Types
+  module AlertManagement
+    class PayloadAlertFieldPathSegmentType < BaseScalar
+      graphql_name 'PayloadAlertFieldPathSegment'
+      description 'String or integer.'
+
+      def self.coerce_input(value, ctx)
+        return value if value.is_a?(::Integer)
+
+        GraphQL::STRING_TYPE.coerce_input(value, ctx)
+      end
+
+      def self.coerce_result(value, ctx)
+        return value if value.is_a?(::Integer)
+
+        GraphQL::STRING_TYPE.coerce_result(value, ctx)
+      end
+    end
+  end
+end

ee/app/graphql/types/alert_management/payload_alert_field_type.rb

@@ -9,7 +9,7 @@ module Types
       authorize :read_alert_management_alert
 
       field :path,
-            [GraphQL::STRING_TYPE],
+            [Types::AlertManagement::PayloadAlertFieldPathSegmentType],
             null: true,
             description: 'Path to value inside payload JSON.'
 

ee/app/graphql/types/alert_management/payload_alert_mapping_field_type.rb

@@ -14,7 +14,7 @@ module Types
             description: 'A GitLab alert field name.'
 
       field :path,
-            [GraphQL::STRING_TYPE],
+            [::Types::AlertManagement::PayloadAlertFieldPathSegmentType],
             null: true,
             description: 'Path to value inside payload JSON.'
 

ee/app/models/alert_management/alert_payload_field.rb

@@ -16,18 +16,22 @@ module AlertManagement
     validates :label, presence: true
     validates :type, inclusion: { in: SUPPORTED_TYPES }
 
-    validate :ensure_path_is_non_empty_list_of_strings
+    validate :valid_path
 
     private
 
-    def ensure_path_is_non_empty_list_of_strings
-      return if path_is_non_empty_list_of_strings?
+    def valid_path
+      return if valid_path?
 
-      errors.add(:path, 'must be a list of strings')
+      errors.add(:path, 'must be a list of strings or integers')
     end
 
-    def path_is_non_empty_list_of_strings?
-      path.is_a?(Array) && !path.empty? && path.all? { |segment| segment.is_a?(String) }
+    def valid_path?
+      path.is_a?(Array) && !path.empty? && valid_path_elements?(path)
+    end
+
+    def valid_path_elements?(path)
+      path.all? { |segment| segment.is_a?(String) || segment.is_a?(Integer) }
     end
   end
 end

ee/changelogs/unreleased/sy-support-arrays-in-custom-mappings.yml

+---
+title: Add support for array indexing to alert integration custom mappings
+merge_request: 58610
+author:
+type: added

ee/lib/gitlab/alert_management/alert_payload_field_extractor.rb

@@ -21,18 +21,44 @@ module Gitlab
         type = type_of(value)
         return unless type
 
-        label = path.last.humanize
-
         ::AlertManagement::AlertPayloadField.new(
           project: project,
           path: path,
-          label: label,
+          label: label_for(path),
           type: type
         )
       end
 
       # Code duplication with Gitlab::InlineHash#merge_keys ahead!
       # See https://gitlab.com/gitlab-org/gitlab/-/issues/299856
+      #
+      # Determines the keys and indicies needed to identify a value
+      # in a hash with nested values.
+      #
+      # Example:
+      # {
+      #   apple: [:a, :b],
+      #   pickle: {
+      #     dill: true
+      #   },
+      #   pear: [{ bosc: 5, bartlett: [1, [2]] }]
+      # }
+      #
+      # Becomes:
+      # [
+      #   [[:apple], [:a, :b]],
+      #   [[:apple, 0], :a],
+      #   [[:apple, 1], :b],
+      #   [[:pickle, :dill], true],
+      #   [[:pear, 0, :bosc], 5]
+      #   [[:pear], [{:bosc=>5, :bartlett=>[1, [2]]}]],
+      #   [[:pear, 0, :bartlett], [1, [2]]],
+      #   [[:pear, 0, :bartlett, 0], 1],
+      #   [[:pear, 0, :bartlett, 1], [2]],
+      #   [[:pear, 0, :bartlett, 1, 0], 2],
+      # ]
+      #
+      # @return Enumerator
       def deep_traverse(hash)
         return to_enum(__method__, hash) unless block_given?
 
@@ -43,6 +69,12 @@ module Gitlab
 
           if value.is_a?(Hash)
             value.each { |k, v| pairs.unshift [key + [k], v] }
+          elsif value.is_a?(Array)
+            yield key, value
+
+            value.each.with_index do |element, index|
+              pairs.unshift [key + [index], element]
+            end
           else
             yield key, value
           end
@@ -59,6 +91,19 @@ module Gitlab
           ::AlertManagement::AlertPayloadField::STRING_TYPE
         end
       end
+
+      # EX) ['first', 'second'] => 'first/second'
+      # EX) ['first', 'second', 0, 1, 'third'] => 'first/second[0][1]/third'
+      #
+      # Assumes first element in path is a string (as we only
+      # expect a Hash as payload input)
+      def label_for(path)
+        path.reduce do |label, element|
+          next "#{label}[#{element}]" if element.is_a?(Integer)
+
+          "#{label}/#{element}"
+        end
+      end
     end
   end
 end

ee/spec/graphql/types/alert_management/payload_alert_field_path_segment_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GitlabSchema.types['PayloadAlertFieldPathSegment'] do
+  specify { expect(described_class.graphql_name).to eq('PayloadAlertFieldPathSegment') }
+
+  describe '.coerce_input' do
+    subject { described_class.coerce_isolated_input(input) }
+
+    context 'with string' do
+      let(:input) { 'string' }
+
+      it { is_expected.to eq input }
+    end
+
+    context 'with integer' do
+      let(:input) { 16 }
+
+      it { is_expected.to eq input }
+    end
+
+    context 'with non-string or integer' do
+      let(:input) { [1, 2, 3] }
+
+      it { is_expected.to eq nil }
+    end
+  end
+
+  describe '.coerce_result' do
+    subject { described_class.coerce_isolated_result(input) }
+
+    context 'with string' do
+      let(:input) { 'string' }
+
+      it { is_expected.to eq input }
+    end
+
+    context 'with integer' do
+      let(:input) { 16 }
+
+      it { is_expected.to eq input }
+    end
+
+    context 'with non-string or integer' do
+      let(:input) { [1, 2, 3] }
+
+      it { is_expected.to eq input.to_s }
+    end
+  end
+end

ee/spec/lib/gitlab/alert_management/alert_payload_field_extractor_spec.rb

@@ -30,21 +30,33 @@ RSpec.describe Gitlab::AlertManagement::AlertPayloadFieldExtractor do
         discarded_null: nil,
         discarded_bool_true: true,
         discarded_bool_false: false,
-        arr: %w[one two three]
+        arr: [
+          { key_a: 'string', key_b: ['nested_arr_value'] },
+          'non_hash_value',
+          [['array_a'], 'array_b']
+        ]
       )
     end
 
     it 'returns all the possible field combination and types suggestions' do
       expect(fields).to contain_exactly(
-        a_field(['str'], 'Str', 'string'),
-        a_field(%w(nested key), 'Key', 'string'),
-        a_field(%w(nested deep key), 'Key', 'string'),
-        a_field(['time'], 'Time', 'datetime'),
-        a_field(['time_iso_8601_and_rfc_3339'], 'Time iso 8601 and rfc 3339', 'datetime'),
-        a_field(['time_iso_8601'], 'Time iso 8601', 'datetime'),
-        a_field(['time_iso_8601_short'], 'Time iso 8601 short', 'datetime'),
-        a_field(['time_rfc_3339'], 'Time rfc 3339', 'datetime'),
-        a_field(['arr'], 'Arr', 'array')
+        a_field(['str'], 'str', 'string'),
+        a_field(%w(nested key), 'nested/key', 'string'),
+        a_field(%w(nested deep key), 'nested/deep/key', 'string'),
+        a_field(['time'], 'time', 'datetime'),
+        a_field(['time_iso_8601_and_rfc_3339'], 'time_iso_8601_and_rfc_3339', 'datetime'),
+        a_field(['time_iso_8601'], 'time_iso_8601', 'datetime'),
+        a_field(['time_iso_8601_short'], 'time_iso_8601_short', 'datetime'),
+        a_field(['time_rfc_3339'], 'time_rfc_3339', 'datetime'),
+        a_field(['arr'], 'arr', 'array'),
+        a_field(['arr', 0, 'key_a'], 'arr[0]/key_a', 'string'),
+        a_field(['arr', 0, 'key_b'], 'arr[0]/key_b', 'array'),
+        a_field(['arr', 0, 'key_b', 0], 'arr[0]/key_b[0]', 'string'),
+        a_field(['arr', 1], 'arr[1]', 'string'),
+        a_field(['arr', 2], 'arr[2]', 'array'),
+        a_field(['arr', 2, 0], 'arr[2][0]', 'array'),
+        a_field(['arr', 2, 1], 'arr[2][1]', 'string'),
+        a_field(['arr', 2, 0, 0], 'arr[2][0][0]', 'string')
       )
     end
   end

ee/spec/models/alert_management/alert_payload_field_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe AlertManagement::AlertPayloadField do
       shared_examples 'has invalid path' do
         it 'is invalid' do
           expect(alert_payload_field.valid?).to eq(false)
-          expect(alert_payload_field.errors.full_messages).to eq(['Path must be a list of strings'])
+          expect(alert_payload_field.errors.full_messages).to eq(['Path must be a list of strings or integers'])
         end
       end
 
@@ -32,11 +32,17 @@ RSpec.describe AlertManagement::AlertPayloadField do
         it_behaves_like 'has invalid path'
       end
 
-      context 'when path does not contain only strings' do
-        let(:alert_payload_field) { build(:alert_management_alert_payload_field, path: ['title', 1]) }
+      context 'when path does not contain only strings or integers' do
+        let(:alert_payload_field) { build(:alert_management_alert_payload_field, path: ['title', {}]) }
 
         it_behaves_like 'has invalid path'
       end
+
+      context 'when path contains only strings and integers' do
+        let(:alert_payload_field) { build(:alert_management_alert_payload_field, path: ['title', 1]) }
+
+        it { is_expected.to be_valid }
+      end
     end
   end
 end

ee/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb

@@ -11,14 +11,17 @@ RSpec.describe 'Creating a new HTTP Integration' do
   let(:payload_example) do
     {
       'alert' => { 'name' => 'Test alert' },
-      'started_at' => Time.current.strftime('%d %B %Y, %-l:%M%p (%Z)')
+      'started_at' => Time.current.strftime('%d %B %Y, %-l:%M%p (%Z)'),
+      'tool' => %w[DataDog V1]
     }.to_json
   end
 
   let(:payload_attribute_mappings) do
     [
       { fieldName: 'TITLE', path: %w[alert name], type: 'STRING' },
-      { fieldName: 'START_TIME', path: %w[started_at], type: 'DATETIME', label: 'Start time' }
+      { fieldName: 'START_TIME', path: %w[started_at], type: 'DATETIME', label: 'Start time' },
+      { fieldName: 'MONITORING_TOOL', path: ['tool', 0], type: 'STRING', label: 'Tool[0]' },
+      { fieldName: 'HOSTS', path: %w[tool], type: 'ARRAY', label: 'Tool' }
     ]
   end
 
@@ -45,6 +48,13 @@ RSpec.describe 'Creating a new HTTP Integration' do
            token
            url
            apiUrl
+           payloadExample
+           payloadAttributeMappings {
+             fieldName
+             path
+             label
+             type
+           }
          }
       QL
     end
@@ -61,9 +71,10 @@ RSpec.describe 'Creating a new HTTP Integration' do
 
       expect(response).to have_gitlab_http_status(:success)
       expect(integration_response['id']).to eq(GitlabSchema.id_from_object(new_integration).to_s)
-
       expect(new_integration.payload_example).to eq({})
       expect(new_integration.payload_attribute_mapping).to eq({})
+      expect(integration_response['payloadExample']).to eq('{}')
+      expect(integration_response['payloadAttributeMappings']).to be_empty
     end
   end
 
@@ -79,14 +90,26 @@ RSpec.describe 'Creating a new HTTP Integration' do
     post_graphql_mutation(mutation, current_user: current_user)
 
     new_integration = ::AlertManagement::HttpIntegration.last!
+    integration_response = mutation_response['integration']
 
     expect(new_integration.payload_example).to eq(Gitlab::Json.parse(payload_example))
     expect(new_integration.payload_attribute_mapping).to eq(
       {
         'title' => { 'path' => %w[alert name], 'type' => 'string', 'label' => nil },
-        'start_time' => { 'path' => %w[started_at], 'type' => 'datetime', 'label' => 'Start time' }
+        'start_time' => { 'path' => %w[started_at], 'type' => 'datetime', 'label' => 'Start time' },
+        'monitoring_tool' => { 'path' => ['tool', 0], 'type' => 'string', 'label' => 'Tool[0]' },
+        'hosts' => { 'path' => %w[tool], 'type' => 'array', 'label' => 'Tool' }
       }
     )
+    expect(integration_response['payloadExample']).to eq(payload_example)
+    expect(integration_response['payloadAttributeMappings']).to eq(
+      [
+        { 'fieldName' => 'TITLE', 'path' => %w[alert name], 'type' => 'STRING', 'label' => nil },
+        { 'fieldName' => 'START_TIME', 'path' => %w[started_at], 'type' => 'DATETIME', 'label' => 'Start time' },
+        { 'fieldName' => 'MONITORING_TOOL', 'path' => ['tool', 0], 'type' => 'STRING', 'label' => 'Tool[0]' },
+        { 'fieldName' => 'HOSTS', 'path' => %w[tool], 'type' => 'ARRAY', 'label' => 'Tool' }
+      ]
+    )
   end
 
   [:project_path, :active, :name].each do |argument|

ee/spec/requests/api/graphql/mutations/alert_management/http_integration/update_spec.rb

@@ -43,6 +43,9 @@ RSpec.describe 'Updating an existing HTTP Integration' do
           payloadExample
           payloadAttributeMappings {
             fieldName
+            path
+            label
+            type
           }
         }
       QL
@@ -55,9 +58,12 @@ RSpec.describe 'Updating an existing HTTP Integration' do
     it 'updates integration without the custom mapping params', :aggregate_failures do
       post_graphql_mutation(mutation, current_user: current_user)
 
+      integration.reload
       integration_response = mutation_response['integration']
 
       expect(response).to have_gitlab_http_status(:success)
+      expect(integration.payload_example).to eq({})
+      expect(integration.payload_attribute_mapping).to eq({})
       expect(integration_response['payloadExample']).to eq('{}')
       expect(integration_response['payloadAttributeMappings']).to be_empty
     end
@@ -76,8 +82,10 @@ RSpec.describe 'Updating an existing HTTP Integration' do
   it 'updates the custom mapping params', :aggregate_failures do
     post_graphql_mutation(mutation, current_user: current_user)
 
-    expect(response).to have_gitlab_http_status(:success)
     integration.reload
+    integration_response = mutation_response['integration']
+
+    expect(response).to have_gitlab_http_status(:success)
     expect(integration.payload_example).to eq(Gitlab::Json.parse(payload_example))
     expect(integration.payload_attribute_mapping).to eq(
       'start_time' => {
@@ -91,6 +99,13 @@ RSpec.describe 'Updating an existing HTTP Integration' do
         'type' => 'string'
       }
     )
+    expect(integration_response['payloadExample']).to eq(payload_example)
+    expect(integration_response['payloadAttributeMappings']).to eq(
+      [
+        { 'fieldName' => 'TITLE', 'path' => %w[alert name], 'type' => 'STRING', 'label' => nil },
+        { 'fieldName' => 'START_TIME', 'path' => %w[started_at], 'type' => 'DATETIME', 'label' => 'Start time' }
+      ]
+    )
   end
 
   context 'when the integration already has custom mapping params' do

ee/spec/requests/api/graphql/project/alert_management/http_integrations_spec.rb

@@ -129,12 +129,12 @@ RSpec.describe 'getting Alert Management HTTP Integrations' do
             ],
             'payloadAlertFields' => [
               {
-                'label' => 'Name',
+                'label' => 'alert/name',
                 'path' => %w(alert name),
                 'type' => 'STRING'
               },
               {
-                'label' => 'Desc',
+                'label' => 'alert/desc',
                 'path' => %w(alert desc),
                 'type' => 'STRING'
               }

ee/spec/requests/api/graphql/project/alert_management/payload_fields_spec.rb

@@ -55,10 +55,12 @@ RSpec.describe 'parse alert payload fields' do
 
   specify do
     expect(parsed_fields).to eq([
-      { 'path' => %w[title], 'label' => 'Title', 'type' => 'STRING' },
-      { 'path' => %w[started_at], 'label' => 'Started at', 'type' => 'DATETIME' },
-      { 'path' => %w[nested key], 'label' => 'Key', 'type' => 'STRING' },
-      { 'path' => %w[arr], 'label' => 'Arr', 'type' => 'ARRAY' }
+      { 'path' => %w[title], 'label' => 'title', 'type' => 'STRING' },
+      { 'path' => %w[started_at], 'label' => 'started_at', 'type' => 'DATETIME' },
+      { 'path' => %w[nested key], 'label' => 'nested/key', 'type' => 'STRING' },
+      { 'path' => %w[arr], 'label' => 'arr', 'type' => 'ARRAY' },
+      { 'path' => ['arr', 1], 'label' => 'arr[1]', 'type' => 'STRING' },
+      { 'path' => ['arr', 0], 'label' => 'arr[0]', 'type' => 'STRING' }
     ])
   end
 

ee/spec/services/alert_management/extract_alert_payload_fields_service_spec.rb

@@ -38,7 +38,7 @@ RSpec.describe AlertManagement::ExtractAlertPayloadFieldsService do
             field = fields.first
 
             expect(fields.count).to eq(1)
-            expect(field.label).to eq('Foo')
+            expect(field.label).to eq('foo')
             expect(field.type).to eq('string')
             expect(field.path).to eq(%w[foo])
           end