From 5e74cebe9a82e14b171e59c0c12627fccbe29ff3 Mon Sep 17 00:00:00 2001 From: Can Eldem <celdem@gitlab.com> Date: Tue, 26 Jan 2021 12:12:12 +0000 Subject: [PATCH] Add parser for cilium alerts Cilium alerts were using generic parser This parser is created to parsing alert coming from KAS --- .../alert_management/network_alert_service.rb | 10 +--- ee/lib/ee/gitlab/alert_management/payload.rb | 26 ++++++++++ .../gitlab/alert_management/payload/cilium.rb | 19 ++++++++ ee/spec/factories/alert_management/alerts.rb | 29 +++++++++++ .../gitlab/alert_management/payload_spec.rb | 23 +++++++++ .../alert_management/payload/cilium_spec.rb | 18 +++++++ .../requests/api/internal/kubernetes_spec.rb | 3 +- .../network_alert_service_spec.rb | 48 +++++++++---------- lib/gitlab/alert_management/payload.rb | 2 + 9 files changed, 142 insertions(+), 36 deletions(-) create mode 100644 ee/lib/ee/gitlab/alert_management/payload.rb create mode 100644 ee/lib/gitlab/alert_management/payload/cilium.rb create mode 100644 ee/spec/factories/alert_management/alerts.rb create mode 100644 ee/spec/lib/ee/gitlab/alert_management/payload_spec.rb create mode 100644 ee/spec/lib/gitlab/alert_management/payload/cilium_spec.rb diff --git a/ee/app/services/alert_management/network_alert_service.rb b/ee/app/services/alert_management/network_alert_service.rb index 144d752be8fca..655472a34c310 100644 --- a/ee/app/services/alert_management/network_alert_service.rb +++ b/ee/app/services/alert_management/network_alert_service.rb @@ -4,7 +4,6 @@ module AlertManagement # Create alerts coming K8 through gitlab-agent class NetworkAlertService include Gitlab::Utils::StrongMemoize - include ::IncidentManagement::Settings MONITORING_TOOL = Gitlab::AlertManagement::Payload::MONITORING_TOOLS.fetch(:cilium) @@ -18,8 +17,6 @@ def initialize(project, payload) def execute return bad_request unless valid_payload_size? - # Not meant to run with a user, but with a agent - # See https://gitlab.com/gitlab-org/gitlab/-/issues/291986 process_request return bad_request unless alert.persisted? @@ -80,14 +77,9 @@ def build_new_alert AlertManagement::Alert.new(**incoming_payload.alert_params, domain: :threat_monitoring, ended_at: nil) end - # https://gitlab.com/gitlab-org/gitlab/-/issues/292034 def incoming_payload strong_memoize(:incoming_payload) do - Gitlab::AlertManagement::Payload.parse( - project, - payload, - monitoring_tool: MONITORING_TOOL - ) + Gitlab::AlertManagement::Payload.parse(project, payload, monitoring_tool: MONITORING_TOOL) end end diff --git a/ee/lib/ee/gitlab/alert_management/payload.rb b/ee/lib/ee/gitlab/alert_management/payload.rb new file mode 100644 index 0000000000000..6c6c18c7ca847 --- /dev/null +++ b/ee/lib/ee/gitlab/alert_management/payload.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module EE + module Gitlab + module AlertManagement + module Payload + extend ActiveSupport::Concern + + class_methods do + extend ::Gitlab::Utils::Override + + private + + override :payload_class_for + def payload_class_for(monitoring_tool:, payload:) + if monitoring_tool == ::Gitlab::AlertManagement::Payload::MONITORING_TOOLS[:cilium] + ::Gitlab::AlertManagement::Payload::Cilium + else + super + end + end + end + end + end + end +end diff --git a/ee/lib/gitlab/alert_management/payload/cilium.rb b/ee/lib/gitlab/alert_management/payload/cilium.rb new file mode 100644 index 0000000000000..c80ec30625a1e --- /dev/null +++ b/ee/lib/gitlab/alert_management/payload/cilium.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + module AlertManagement + module Payload + class Cilium < Gitlab::AlertManagement::Payload::Generic + DEFAULT_TITLE = 'New: Alert' + + attribute :description, paths: %w(flow dropReasonDesc) + attribute :title, paths: %w(ciliumNetworkPolicy metadata name), fallback: -> { DEFAULT_TITLE } + attribute :gitlab_fingerprint, paths: %w(fingerprint) + + def monitoring_tool + Gitlab::AlertManagement::Payload::MONITORING_TOOLS[:cilium] + end + end + end + end +end diff --git a/ee/spec/factories/alert_management/alerts.rb b/ee/spec/factories/alert_management/alerts.rb new file mode 100644 index 0000000000000..41887b44da1e2 --- /dev/null +++ b/ee/spec/factories/alert_management/alerts.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :network_alert_payload, class: Hash do + initialize_with do + { + fingerprint: 'a94a8fe5ccb19ba61c4c0873d391e987982fbbd3', + flow: { + dropReasonDesc: "POLICY_DENIED" + }, + ciliumNetworkPolicy: { + kind: 'bla', + apiVersion: 'bla', + metadata: { + name: 'Cilium Alert', + generateName: 'generated NAme', + namespace: 'LocalGitlab', + selfLink: 'www.gitlab.com', + uid: '2d931510-d99f-494a-8c67-87feb05e1594', + resourceVersion: '23', + deletionGracePeriodSeconds: 42, + clusterName: 'TestCluster' + }, + status: {} + } + }.with_indifferent_access + end + end +end diff --git a/ee/spec/lib/ee/gitlab/alert_management/payload_spec.rb b/ee/spec/lib/ee/gitlab/alert_management/payload_spec.rb new file mode 100644 index 0000000000000..26930044b6a32 --- /dev/null +++ b/ee/spec/lib/ee/gitlab/alert_management/payload_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::AlertManagement::Payload do + describe '#parse' do + let_it_be(:project) { build_stubbed(:project) } + let(:payload) { {} } + + context 'with the payload specifing cilium as monitoring tool' do + before do + stub_licensed_features(cilium_alerts: true) + end + subject { described_class.parse(project, payload) } + + context 'with the payload specifying an unknown tool' do + let(:payload) { { 'monitoring_tool' => 'Cilium' } } + + it { is_expected.to be_a Gitlab::AlertManagement::Payload::Cilium } + end + end + end +end diff --git a/ee/spec/lib/gitlab/alert_management/payload/cilium_spec.rb b/ee/spec/lib/gitlab/alert_management/payload/cilium_spec.rb new file mode 100644 index 0000000000000..6cf3e28a16d26 --- /dev/null +++ b/ee/spec/lib/gitlab/alert_management/payload/cilium_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::AlertManagement::Payload::Cilium do + let_it_be(:project) { build_stubbed(:project) } + let(:raw_payload) { build(:network_alert_payload).to_json } + + let(:parsed_payload) do + described_class.new(project: project, payload: Gitlab::Json.parse(raw_payload)) + end + + it 'parses cilium specific fields' do + expect(parsed_payload.title).to eq('Cilium Alert') + expect(parsed_payload.description).to eq('POLICY_DENIED') + expect(parsed_payload.gitlab_fingerprint).to eq('a94a8fe5ccb19ba61c4c0873d391e987982fbbd3') + end +end diff --git a/ee/spec/requests/api/internal/kubernetes_spec.rb b/ee/spec/requests/api/internal/kubernetes_spec.rb index 31a7374a0d781..7adcf7f8d1838 100644 --- a/ee/spec/requests/api/internal/kubernetes_spec.rb +++ b/ee/spec/requests/api/internal/kubernetes_spec.rb @@ -71,8 +71,7 @@ def send_request(headers: {}, params: {}) { alert: { title: 'minimal', - message: 'network problem', - evalMatches: [{ value: 1, metric: 'Count', tags: {} }] + message: 'network problem' } } end diff --git a/ee/spec/services/alert_management/network_alert_service_spec.rb b/ee/spec/services/alert_management/network_alert_service_spec.rb index b4c35e583fe8e..e4f566bcd3a6f 100644 --- a/ee/spec/services/alert_management/network_alert_service_spec.rb +++ b/ee/spec/services/alert_management/network_alert_service_spec.rb @@ -37,20 +37,7 @@ end context 'with valid payload' do - let(:payload_raw) do - { - title: 'alert title', - start_time: starts_at.rfc3339, - end_time: ended_at&.rfc3339, - severity: 'low', - monitoring_tool: tool, - service: 'GitLab Test Suite', - description: 'Very detailed description', - hosts: %w[1.1.1.1 2.2.2.2], - fingerprint: fingerprint, - gitlab_environment_name: environment.name - }.with_indifferent_access - end + let(:payload_raw) { build(:network_alert_payload) } let(:payload) { ActionController::Parameters.new(payload_raw).permit! } @@ -59,12 +46,31 @@ .with_indifferent_access end - it_behaves_like 'creates an alert management alert' - it_behaves_like 'assigns the alert properties' + it 'create alert and assigns properties' do + subject + + expect(last_alert_attributes).to match(a_hash_including({ + description: 'POLICY_DENIED', + domain: 'threat_monitoring', + ended_at: nil, + environment_id: nil, + events: 1, + fingerprint: 'a94a8fe5ccb19ba61c4c0873d391e987982fbbd3', + hosts: [], + issue_id: nil, + monitoring_tool: 'Cilium', + payload: payload_raw.with_indifferent_access, + project_id: project.id, + prometheus_alert_id: nil, + service: nil, + severity: 'critical', + title: 'Cilium Alert' + })) + end it 'creates a system note corresponding to alert creation' do expect { subject }.to change(Note, :count).by(1) - expect(Note.last.note).to include(payload_raw.fetch(:monitoring_tool)) + expect(Note.last.note).to include('Cilium') end context 'when alert exists' do @@ -116,7 +122,6 @@ end it_behaves_like 'creates an alert management alert' - it_behaves_like 'assigns the alert properties' end context 'existing alert is ignored' do @@ -147,13 +152,6 @@ it_behaves_like 'adds an alert management alert event' end end - - context 'end time given' do - let(:ended_at) { Time.current } - - it_behaves_like 'creates an alert management alert' - it_behaves_like 'assigns the alert properties' - end end context 'with overlong payload' do diff --git a/lib/gitlab/alert_management/payload.rb b/lib/gitlab/alert_management/payload.rb index ce09ffd87ee4e..d3ce5cc8c74ef 100644 --- a/lib/gitlab/alert_management/payload.rb +++ b/lib/gitlab/alert_management/payload.rb @@ -47,3 +47,5 @@ def gitlab_managed_prometheus?(payload) end end end + +Gitlab::AlertManagement::Payload.prepend_if_ee('EE::Gitlab::AlertManagement::Payload') -- GitLab