Commit 80b81475 authored by Sarah Yasonik's avatar Sarah Yasonik Committed by Peter Leitzen

Truncate AlertManagement::Alert fields which may be too long

parent 2238aa7a
......@@ -38,12 +38,16 @@ module AlertManagement
sha_attribute :fingerprint
TITLE_MAX_LENGTH = 200
DESCRIPTION_MAX_LENGTH = 1_000
SERVICE_MAX_LENGTH = 100
TOOL_MAX_LENGTH = 100
HOSTS_MAX_LENGTH = 255
validates :title, length: { maximum: 200 }, presence: true
validates :description, length: { maximum: 1_000 }
validates :service, length: { maximum: 100 }
validates :monitoring_tool, length: { maximum: 100 }
validates :title, length: { maximum: TITLE_MAX_LENGTH }, presence: true
validates :description, length: { maximum: DESCRIPTION_MAX_LENGTH }
validates :service, length: { maximum: SERVICE_MAX_LENGTH }
validates :monitoring_tool, length: { maximum: TOOL_MAX_LENGTH }
validates :project, presence: true
validates :events, presence: true
validates :severity, presence: true
......@@ -54,7 +58,7 @@ module AlertManagement
conditions: -> { not_resolved },
message: -> (object, data) { _('Cannot have multiple unresolved alerts') }
}, unless: :resolved?
validate :hosts_length
validate :hosts_format
enum severity: {
critical: 0,
......@@ -251,10 +255,11 @@ module AlertManagement
Gitlab::DataBuilder::Alert.build(self)
end
def hosts_length
def hosts_format
return unless hosts
errors.add(:hosts, "hosts array is over #{HOSTS_MAX_LENGTH} chars") if hosts.join.length > HOSTS_MAX_LENGTH
errors.add(:hosts, "hosts array cannot be nested") if hosts.flatten != hosts
end
end
end
---
title: Truncate over-long alert fields instead of return error response
merge_request: 45099
author:
type: changed
......@@ -88,19 +88,19 @@ module Gitlab
# AlertManagement::Alert directly for read operations.
def alert_params
{
description: description,
description: description&.truncate(::AlertManagement::Alert::DESCRIPTION_MAX_LENGTH),
ended_at: ends_at,
environment: environment,
fingerprint: gitlab_fingerprint,
hosts: Array(hosts),
monitoring_tool: monitoring_tool,
hosts: truncate_hosts(Array(hosts).flatten),
monitoring_tool: monitoring_tool&.truncate(::AlertManagement::Alert::TOOL_MAX_LENGTH),
payload: payload,
project_id: project.id,
prometheus_alert: gitlab_alert,
service: service,
service: service&.truncate(::AlertManagement::Alert::SERVICE_MAX_LENGTH),
severity: severity,
started_at: starts_at,
title: title
title: title&.truncate(::AlertManagement::Alert::TITLE_MAX_LENGTH)
}.transform_values(&:presence).compact
end
......@@ -135,6 +135,18 @@ module Gitlab
def plain_gitlab_fingerprint; end
def truncate_hosts(hosts)
return hosts if hosts.join.length <= ::AlertManagement::Alert::HOSTS_MAX_LENGTH
hosts.inject([]) do |new_hosts, host|
remaining_length = ::AlertManagement::Alert::HOSTS_MAX_LENGTH - new_hosts.join.length
break new_hosts unless remaining_length > 0
new_hosts << host.to_s.truncate(remaining_length, omission: '')
end
end
def value_for_paths(paths)
target_path = paths.find { |path| payload&.dig(*path) }
......
......@@ -120,14 +120,107 @@ RSpec.describe Gitlab::AlertManagement::Payload::Base do
end
describe '#alert_params' do
before do
allow(parsed_payload).to receive(:title).and_return('title')
allow(parsed_payload).to receive(:description).and_return('description')
subject { parsed_payload.alert_params }
context 'with every key' do
let_it_be(:raw_payload) { { 'key' => 'value' } }
let_it_be(:stubs) do
{
description: 'description',
ends_at: Time.current,
environment: create(:environment, project: project),
gitlab_fingerprint: 'gitlab_fingerprint',
hosts: 'hosts',
monitoring_tool: 'monitoring_tool',
gitlab_alert: create(:prometheus_alert, project: project),
service: 'service',
severity: 'critical',
starts_at: Time.current,
title: 'title'
}
end
let(:expected_result) do
{
description: stubs[:description],
ended_at: stubs[:ends_at],
environment: stubs[:environment],
fingerprint: stubs[:gitlab_fingerprint],
hosts: [stubs[:hosts]],
monitoring_tool: stubs[:monitoring_tool],
payload: raw_payload,
project_id: project.id,
prometheus_alert: stubs[:gitlab_alert],
service: stubs[:service],
severity: stubs[:severity],
started_at: stubs[:starts_at],
title: stubs[:title]
}
end
before do
allow(parsed_payload).to receive_messages(stubs)
end
it { is_expected.to eq(expected_result) }
it 'can generate a valid new alert' do
expect(::AlertManagement::Alert.new(subject.except(:ended_at))).to be_valid
end
end
subject { parsed_payload.alert_params }
context 'with too-long strings' do
let_it_be(:stubs) do
{
description: 'a' * (::AlertManagement::Alert::DESCRIPTION_MAX_LENGTH + 1),
hosts: 'b' * (::AlertManagement::Alert::HOSTS_MAX_LENGTH + 1),
monitoring_tool: 'c' * (::AlertManagement::Alert::TOOL_MAX_LENGTH + 1),
service: 'd' * (::AlertManagement::Alert::SERVICE_MAX_LENGTH + 1),
title: 'e' * (::AlertManagement::Alert::TITLE_MAX_LENGTH + 1)
}
end
it { is_expected.to eq({ description: 'description', project_id: project.id, title: 'title' }) }
before do
allow(parsed_payload).to receive_messages(stubs)
end
it do
is_expected.to eq({
description: stubs[:description].truncate(AlertManagement::Alert::DESCRIPTION_MAX_LENGTH),
hosts: ['b' * ::AlertManagement::Alert::HOSTS_MAX_LENGTH],
monitoring_tool: stubs[:monitoring_tool].truncate(::AlertManagement::Alert::TOOL_MAX_LENGTH),
service: stubs[:service].truncate(::AlertManagement::Alert::SERVICE_MAX_LENGTH),
project_id: project.id,
title: stubs[:title].truncate(::AlertManagement::Alert::TITLE_MAX_LENGTH)
})
end
end
context 'with too-long hosts array' do
let(:hosts) { %w(abc def ghij) }
let(:shortened_hosts) { %w(abc def ghi) }
before do
stub_const('::AlertManagement::Alert::HOSTS_MAX_LENGTH', 9)
allow(parsed_payload).to receive(:hosts).and_return(hosts)
end
it { is_expected.to eq(hosts: shortened_hosts, project_id: project.id) }
context 'with host cut off between elements' do
let(:hosts) { %w(abcde fghij) }
let(:shortened_hosts) { %w(abcde fghi) }
it { is_expected.to eq({ hosts: shortened_hosts, project_id: project.id }) }
end
context 'with nested hosts' do
let(:hosts) { ['abc', ['de', 'f'], 'g', 'hij'] } # rubocop:disable Style/WordArray
let(:shortened_hosts) { %w(abc de f g hi) }
it { is_expected.to eq({ hosts: shortened_hosts, project_id: project.id }) }
end
end
end
describe '#gitlab_fingerprint' do
......
......@@ -170,6 +170,12 @@ RSpec.describe AlertManagement::Alert do
it { is_expected.to be_valid }
end
context 'nested array' do
let(:hosts) { ['111.111.111.111', ['111.111.111.111']] }
it { is_expected.not_to be_valid }
end
end
end
......
......@@ -117,15 +117,19 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
end
context 'when alert cannot be created' do
let(:errors) { double(messages: { hosts: ['hosts array is over 255 chars'] })}
before do
payload['annotations']['title'] = 'description' * 50
allow(service).to receive(:alert).and_call_original
allow(service).to receive_message_chain(:alert, :save).and_return(false)
allow(service).to receive_message_chain(:alert, :errors).and_return(errors)
end
it 'writes a warning to the log' do
expect(Gitlab::AppLogger).to receive(:warn).with(
message: 'Unable to create AlertManagement::Alert',
project_id: project.id,
alert_errors: { title: ["is too long (maximum is 200 characters)"] }
alert_errors: { hosts: ['hosts array is over 255 chars'] }
)
execute
......
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