fix(genericapi): update summary, details and metadata on dedup match#4514
Open
C2L2C wants to merge 2 commits into
Open
fix(genericapi): update summary, details and metadata on dedup match#4514C2L2C wants to merge 2 commits into
C2L2C wants to merge 2 commits into
Conversation
Previously the generic ingestion API returned IsNew:false when a dedup key matched an existing alert but silently discarded the incoming summary, details and metadata. - Run UPDATE after dedup SELECT so summary/details are persisted - Remove isNew guard on SetMetadataTx so metadata updates too - Add escalate param (form/query/JSON) to re-trigger escalation policy - Extend EscalateAsOf to accept service permission with ownership check Signed-off-by: Aditya Sharma <glennadi007@gmail.com>
C2L2C
force-pushed
the
fix/generic-api-update-and-escalate
branch
from
b85b235 to
6e4d2bf
Compare
Previously meta was initialized as make(map[string]string) in the handler (never nil), so the nil check always passed and SetMetadataTx was called on every request including closes. This caused a 403 when closing an alert because SetMetadataTx returns access denied when the alert is already closed. Fix by checking len(meta) > 0 instead of meta != nil. Signed-off-by: Aditya Sharma <glennadi007@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #4374
When the Generic Alert Ingestion API receives a POST with a dedup key matching an existing alert, it returns
200 IsNew:falsebut silently discards the incomingsummary,details, andmetadata.Root Cause
Two bugs in
alert/store.go:CreateOrUpdateTx— thecreateUpdNewSQL CTE returns the existing row unchanged. Newsummary/detailsare only used in the INSERT branch; the existing-alert branch ignores them entirely.createOrUpdate—SetMetadataTxwas guarded by&& isNew, so metadata was only written for brand-new alerts.Changes
alert/store.go: After a dedup match on a non-closed alert, run a second UPDATE statement to apply newsummary/details. Remove&& isNewguard on metadata so it updates for existing alerts too.alert/store.go: ExtendEscalateAsOfto acceptpermission.Servicewith an ownership check, so services can re-escalate their own alerts via the API.genericapi/handler.go: Addescalateparameter (form value, query string, or JSON body). Whentrueon an existing alert, re-triggers the escalation policy.test/smoke/genericapiupdate_test.go: Smoke test covering summary + metadata update via dedup key, and re-escalation viaescalate:true.Testing
Notes
"escalate":true(boolean). Form-encoded requests acceptescalate=true(string).