Commit c049ff44 authored by Sean Arnold's avatar Sean Arnold Committed by Peter Leitzen

Add fingerprinting for entire payloads

- Feature must be enabled
- Sort hashes
parent befe8808
...@@ -76,7 +76,7 @@ module Projects ...@@ -76,7 +76,7 @@ module Projects
end end
def parsed_payload def parsed_payload
Gitlab::Alerting::NotificationPayloadParser.call(params.to_h) Gitlab::Alerting::NotificationPayloadParser.call(params.to_h, project)
end end
def valid_token?(token) def valid_token?(token)
......
...@@ -74,6 +74,7 @@ class License < ApplicationRecord ...@@ -74,6 +74,7 @@ class License < ApplicationRecord
file_locks file_locks
geo geo
github_project_service_integration github_project_service_integration
generic_alert_fingerprinting
group_allowed_email_domains group_allowed_email_domains
group_ip_restriction group_ip_restriction
group_project_templates group_project_templates
......
# frozen_string_literal: true
module EE
module Gitlab
module Alerting
module NotificationPayloadParser
extend ::Gitlab::Utils::Override
EXCLUDED_PAYLOAD_FINGERPRINT_PARAMS = %w(start_time hosts).freeze
# Currently we use full payloads, when generating a fingerprint.
# This results in a quite strict fingerprint.
# Over time we can relax these rules.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/214557#note_362795447
override :fingerprint
def fingerprint
return super if payload[:fingerprint].present? || !generic_alert_fingerprinting_enabled?
payload_excluding_params = payload.excluding(EXCLUDED_PAYLOAD_FINGERPRINT_PARAMS)
return if payload_excluding_params.none? { |_, v| v.present? }
::Gitlab::AlertManagement::Fingerprint.generate(payload_excluding_params)
end
private
def generic_alert_fingerprinting_enabled?
project.feature_available?(:generic_alert_fingerprinting) &&
project.beta_feature_available?(:generic_alert_fingerprinting)
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Alerting::NotificationPayloadParser do
let(:project) { build_stubbed(:project) }
describe '.call' do
subject(:parsed) { described_class.call(payload, project) }
let(:payload) do
{
'title' => 'alert title',
'start_time' => Time.current,
'description' => 'Description',
'monitoring_tool' => 'Monitoring tool name',
'service' => 'Service',
'hosts' => ['gitlab.com'],
'severity' => 'low'
}
end
describe 'fingerprint' do
subject(:fingerprint) { parsed.dig('annotations', 'fingerprint') }
context 'license feature enabled' do
before do
stub_licensed_features(generic_alert_fingerprinting: true)
end
it 'generates the fingerprint from the payload' do
fingerprint_payload = payload.excluding('start_time', 'hosts')
expected_fingerprint = Gitlab::AlertManagement::Fingerprint.generate(fingerprint_payload)
expect(fingerprint).to eq(expected_fingerprint)
end
context 'feature not enabled' do
before do
stub_feature_flags(generic_alert_fingerprinting: false)
end
it { is_expected.to eq(nil) }
end
context 'payload has no values' do
let(:payload) do
{
'start_time' => Time.current,
'hosts' => ['gitlab.com'],
'title' => ' '
}
end
it { is_expected.to eq(nil) }
end
end
context 'license feature not enabled' do
it { is_expected.to eq(nil) }
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Projects::Alerting::NotifyService do
let_it_be(:project, refind: true) { create(:project) }
describe '#execute' do
let(:service) { described_class.new(project, nil, payload) }
let(:token) { alerts_service.token }
let(:payload) do
{
title: 'Test alert title'
}
end
let(:alerts_service) { create(:alerts_service, project: project) }
subject { service.execute(token) }
context 'existing alert with same payload fingerprint' do
let(:existing_alert) do
service.execute(token)
AlertManagement::Alert.last!
end
before do
stub_licensed_features(generic_alert_fingerprinting: fingerprinting_enabled)
existing_alert # create existing alert
end
context 'generic fingerprinting feature not enabled' do
let(:fingerprinting_enabled) { false }
it 'creates AlertManagement::Alert' do
expect { subject }.to change(AlertManagement::Alert, :count)
end
it 'does not increment the existing alert count' do
expect { subject }.not_to change { existing_alert.reload.events }
end
end
context 'generic fingerprinting feature enabled' do
let(:fingerprinting_enabled) { true }
it 'does not create AlertManagement::Alert' do
expect { subject }.not_to change(AlertManagement::Alert, :count)
end
it 'increments the existing alert count' do
expect { subject }.to change { existing_alert.reload.events }.from(1).to(2)
end
end
end
end
end
...@@ -8,7 +8,7 @@ module Gitlab ...@@ -8,7 +8,7 @@ module Gitlab
}.freeze }.freeze
def self.from_generic_alert(project:, payload:) def self.from_generic_alert(project:, payload:)
parsed_payload = Gitlab::Alerting::NotificationPayloadParser.call(payload).with_indifferent_access parsed_payload = Gitlab::Alerting::NotificationPayloadParser.call(payload, project).with_indifferent_access
annotations = parsed_payload[:annotations] annotations = parsed_payload[:annotations]
{ {
......
...@@ -10,11 +10,14 @@ module Gitlab ...@@ -10,11 +10,14 @@ module Gitlab
def generate(data) def generate(data)
return unless data.present? return unless data.present?
if data.is_a?(Array) string = case data
data = flatten_array(data) when Array then flatten_array(data)
when Hash then flatten_hash(data)
else
data.to_s
end end
Digest::SHA1.hexdigest(data.to_s) Digest::SHA1.hexdigest(string)
end end
private private
...@@ -22,6 +25,11 @@ module Gitlab ...@@ -22,6 +25,11 @@ module Gitlab
def flatten_array(array) def flatten_array(array)
array.flatten.map!(&:to_s).join array.flatten.map!(&:to_s).join
end end
def flatten_hash(hash)
# Sort hash so SHA generated is the same
Gitlab::Utils::SafeInlineHash.merge_keys!(hash).sort.to_s
end
end end
end end
end end
...@@ -8,12 +8,13 @@ module Gitlab ...@@ -8,12 +8,13 @@ module Gitlab
DEFAULT_TITLE = 'New: Incident' DEFAULT_TITLE = 'New: Incident'
DEFAULT_SEVERITY = 'critical' DEFAULT_SEVERITY = 'critical'
def initialize(payload) def initialize(payload, project)
@payload = payload.to_h.with_indifferent_access @payload = payload.to_h.with_indifferent_access
@project = project
end end
def self.call(payload) def self.call(payload, project)
new(payload).call new(payload, project).call
end end
def call def call
...@@ -25,7 +26,7 @@ module Gitlab ...@@ -25,7 +26,7 @@ module Gitlab
private private
attr_reader :payload attr_reader :payload, :project
def title def title
payload[:title].presence || DEFAULT_TITLE payload[:title].presence || DEFAULT_TITLE
...@@ -84,3 +85,5 @@ module Gitlab ...@@ -84,3 +85,5 @@ module Gitlab
end end
end end
end end
Gitlab::Alerting::NotificationPayloadParser.prepend_if_ee('EE::Gitlab::Alerting::NotificationPayloadParser')
...@@ -13,28 +13,18 @@ RSpec.describe Gitlab::AlertManagement::Fingerprint do ...@@ -13,28 +13,18 @@ RSpec.describe Gitlab::AlertManagement::Fingerprint do
context 'when data is an array' do context 'when data is an array' do
let(:data) { [1, 'fingerprint', 'given'] } let(:data) { [1, 'fingerprint', 'given'] }
it 'flattens the array' do
expect_next_instance_of(described_class) do |obj|
expect(obj).to receive(:flatten_array)
end
subject
end
it 'returns the hashed fingerprint' do it 'returns the hashed fingerprint' do
expected_fingerprint = Digest::SHA1.hexdigest(data.flatten.map!(&:to_s).join) expected_fingerprint = Digest::SHA1.hexdigest(data.flatten.map!(&:to_s).join)
expect(subject).to eq(expected_fingerprint) expect(subject).to eq(expected_fingerprint)
end end
end
context 'when data is a non-array type' do context 'with a variety of data' do
where(:data) do where(:data) do
[ [
111, 111,
'fingerprint', 'fingerprint',
:fingerprint, :fingerprint,
true, true
{ test: true }
] ]
end end
...@@ -45,4 +35,42 @@ RSpec.describe Gitlab::AlertManagement::Fingerprint do ...@@ -45,4 +35,42 @@ RSpec.describe Gitlab::AlertManagement::Fingerprint do
end end
end end
end end
context 'when data is a hash' do
let(:data) { { test: true } }
shared_examples 'fingerprinted Hash' do
it 'performs like a hashed fingerprint' do
flattened_hash = Gitlab::Utils::SafeInlineHash.merge_keys!(data).sort.to_s
expect(subject).to eq(Digest::SHA1.hexdigest(flattened_hash))
end
end
it_behaves_like 'fingerprinted Hash'
context 'hashes with different order' do
it 'calculates the same result' do
data = { test: true, another_test: 1 }
data_hash = described_class.generate(data)
reverse_data = { another_test: 1, test: true }
reverse_data_hash = described_class.generate(reverse_data)
expect(data_hash).to eq(reverse_data_hash)
end
end
context 'hash is too large' do
before do
expect_next_instance_of(Gitlab::Utils::SafeInlineHash) do |obj|
expect(obj).to receive(:valid?).and_return(false)
end
end
it 'raises an error' do
expect { subject }.to raise_error(ArgumentError)
end
end
end
end
end end
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Alerting::NotificationPayloadParser do RSpec.describe Gitlab::Alerting::NotificationPayloadParser do
let_it_be(:project) { build(:project) }
describe '.call' do describe '.call' do
let(:starts_at) { Time.current.change(usec: 0) } let(:starts_at) { Time.current.change(usec: 0) }
let(:payload) do let(:payload) do
...@@ -17,7 +19,7 @@ RSpec.describe Gitlab::Alerting::NotificationPayloadParser do ...@@ -17,7 +19,7 @@ RSpec.describe Gitlab::Alerting::NotificationPayloadParser do
} }
end end
subject { described_class.call(payload) } subject { described_class.call(payload, project) }
it 'returns Prometheus-like payload' do it 'returns Prometheus-like payload' do
is_expected.to eq( is_expected.to eq(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment