Skip to content

fix: preserve subject and source on GenericChange findings - #2129

Open
Sanjays2402 wants to merge 2 commits into
robusta-dev:masterfrom
Sanjays2402:fix/generic-change-finding-subject-2044
Open

fix: preserve subject and source on GenericChange findings#2129
Sanjays2402 wants to merge 2 commits into
robusta-dev:masterfrom
Sanjays2402:fix/generic-change-finding-subject-2044

Conversation

@Sanjays2402

Copy link
Copy Markdown

Closes #2044

K8sBaseChangeEvent.create_default_finding() built a Finding with only a title and aggregation_key, so finding.subject stayed at the ExecutionBaseEvent default (namespace None) and source stayed NONE. Sink routing reads finding.attribute_map["namespace"], which derives from subject.namespace, so Kubernetes change notifications were routed as namespace "None" rather than the resource's real namespace.

The event already exposes get_subject()/get_source() — this passes them through, matching what PrometheusKubernetesAlert.create_default_finding() already does. The new test in tests/test_change_filters.py fails on master (assert None == 'default') and passes with the fix.

K8sBaseChangeEvent.create_default_finding() built a Finding with only a
title and aggregation_key, so finding.subject stayed at the ExecutionBaseEvent
default (name "Unresolved", namespace None) and source stayed NONE.

Sink routing and Slack grouping read finding.attribute_map["namespace"],
which derives from finding.subject.namespace, so Kubernetes change
notifications were routed as namespace "None" instead of the resource's
actual namespace - sending production changes to a fallback sink.

The event already exposes get_subject() and get_source(); pass them through,
matching how PrometheusKubernetesAlert.create_default_finding() already does.

Adds a regression test asserting the default finding keeps the resource's
namespace, name and KUBERNETES_API_SERVER source.
@CLAassistant

CLAassistant commented Jul 26, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 46c9cfd6-c9e7-4284-8494-d84ceccf5e07

📥 Commits

Reviewing files that changed from the base of the PR and between 1479d6f and 312869f.

📒 Files selected for processing (1)
  • tests/test_change_filters.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/test_change_filters.py

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

Kubernetes default findings now include the event subject and source. Tests cover aggregation key, namespace/name preservation, Kubernetes API server source, and corresponding attribute map values.

Changes

Kubernetes finding metadata

Layer / File(s) Summary
Default finding metadata and regression coverage
src/robusta/integrations/kubernetes/base_event.py, tests/test_change_filters.py
create_default_finding() now sets the event subject and source. Tests verify the aggregation key, subject fields, source, and namespace/name attributes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 31286

Kubernetes GenericChange findings now retain resource subject metadata and Kubernetes API-server source, preserving namespace-aware routing and grouping. Regression coverage accompanies the change, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preserving subject and source metadata on GenericChange findings.
Description check ✅ Passed The description directly explains the Kubernetes finding metadata bug, its routing impact, the fix, and the regression test.
Linked Issues check ✅ Passed The implementation adds get_subject() and get_source() to default Kubernetes findings. The regression test verifies the required subject metadata and KUBERNETES_API_SERVER source for issue #2044.
Out of Scope Changes check ✅ Passed The changes are limited to the issue fix and related regression coverage. The import cleanup and added metadata assertions are in scope.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_change_filters.py`:
- Around line 99-108: Extend test_default_finding_keeps_subject_and_source to
assert that finding.subject.labels and finding.subject.annotations match the
corresponding labels and annotations in the event fixture metadata, alongside
the existing namespace and name checks.
- Line 17: Remove the unused DEFAULT_CHANGE_INCLUDE import from the imports in
tests/test_change_filters.py, leaving the required DEFAULT_CHANGE_IGNORE import
and other dependencies unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ee27cd95-e523-495c-918d-219feaa8db02

📥 Commits

Reviewing files that changed from the base of the PR and between 904bd04 and 1479d6f.

📒 Files selected for processing (2)
  • src/robusta/integrations/kubernetes/base_event.py
  • tests/test_change_filters.py

Comment thread tests/test_change_filters.py
Comment thread tests/test_change_filters.py
@Sanjays2402

Copy link
Copy Markdown
Author

Thanks for the review — both points addressed in 312869f:

  • Dropped the unused DEFAULT_CHANGE_INCLUDE import from tests/test_change_filters.py.
  • Extended test_default_finding_keeps_subject_and_source to assert finding.subject.labels == {"app": "xxx"} and finding.subject.annotations == {"deployment.kubernetes.io/revision": "24"} against the fixture metadata, so a regression in the full subject is caught.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GenericChange Kubernetes notifications can lose subject.namespace and be routed to the wrong sink

2 participants