-
Notifications
You must be signed in to change notification settings - Fork 292
feat: generic objects #1996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat: generic objects #1996
Conversation
WalkthroughAdds support for generic Kubernetes objects through a new wrapper class, enabling graceful handling of unsupported resource kinds without raising exceptions. Introduces subject extraction logic from Kubernetes objects and updates event building workflow. Includes documentation for HTTPRoute monitoring example. Changes
Sequence Diagram(s)sequenceDiagram
participant Input as Kubernetes Event
participant Parser as Object Parser
participant GenericObj as GenericKubernetesObject
participant EventBuilder as Event Builder
participant SubjectExt as Subject Extractor
participant Filter as Event Filter
Input->>Parser: Unsupported Kind Received
alt Model Class Available
Parser->>EventBuilder: Parsed Object
else Model Class Unavailable
Parser->>GenericObj: Create Wrapper (logs warning)
GenericObj->>EventBuilder: Generic Wrapper
end
EventBuilder->>EventBuilder: Create KubernetesAnyChangeEvent
EventBuilder->>SubjectExt: Extract Subject from Object
SubjectExt->>SubjectExt: Derive name, namespace, labels, annotations
SubjectExt->>SubjectExt: Determine subject_type via FindingSubjectType.from_kind
SubjectExt->>Filter: Subject Extracted
alt Generic Object
Filter->>Filter: Bypass duplicate_without_fields
Filter->>Filter: Return with empty filtered_diffs
else Specific Object
Filter->>Filter: Apply standard duplicate filtering
end
Filter->>Output: Event Ready for Processing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@docs/playbook-reference/kubernetes-examples/kubernetes-generic-example.rst`:
- Around line 1-2: The RST title underline is shorter than the title text
"Generic resource tracking"; update the underline row (the line currently
containing `===============`) so it matches or exceeds the title length (at
least 25 characters) by extending the sequence of `=` characters to equal the
title length, ensuring the underline and title use the same adornment character.
- Around line 44-47: The ServiceAccount entry hardcodes namespace: default which
may not match users' Robusta installation; update the documentation snippet
around the subjects -> kind: ServiceAccount / name:
robusta-forwarder-service-account to either remove the hardcoded namespace or
add a clear note and example showing the namespace should be set to the Robusta
namespace (e.g., robusta) or replaced by a placeholder (e.g.,
<ROBUSTA_NAMESPACE>) so users adjust it to their deployment.
- Around line 74-82: The example's Kubernetes filters don't match the HTTPRoute
scenario and include a stray "#" comment; update the customPlaybooks entry for
on_kubernetes_resource_operation so namespace_prefix and name_prefix reflect the
HTTPRoute example (e.g., set namespace_prefix to the HTTPRoute namespace used
earlier and name_prefix to a prefix like "httproute-" or the actual HTTPRoute
name prefix), ensure the create_finding action uses the intended title,
aggregation_key, etc., and remove the trailing "#" after create_finding to
eliminate the incomplete comment.
In `@src/robusta/integrations/kubernetes/base_event.py`:
- Around line 57-61: Replace the bare except around FindingSubjectType.from_kind
by calling it directly (subject_type = FindingSubjectType.from_kind(kind)) since
from_kind already returns TYPE_NONE for unknown kinds; if you prefer defensive
handling, catch only specific exceptions like (ValueError, TypeError) and handle
or log them rather than using a bare except, ensuring subject_type falls back to
FindingSubjectType.TYPE_NONE.
🧹 Nitpick comments (2)
src/robusta/integrations/kubernetes/base_triggers.py (2)
30-35: Dynamic class creation works but is unconventional.Using
type('Metadata', (), {...})()creates a one-off class instance. This works but can make debugging harder (class name always shows asMetadata) and doesn't provide type hints. Consider using a simple dataclass or named tuple for clarity, though this is a minor concern.
22-39: Improve semantic correctness of attribute access fallback inGenericKubernetesObject.The
__getattr__method returnsNonefor missing attributes instead of raisingAttributeError. While the codebase currently handles this correctly (by checking bothhasattr()and truthiness), this violates Python conventions. The code containshasattr()checks on objects that could beGenericKubernetesObjectinstances (e.g.,base_event.pyline 19, 41;finding_subjects.pyline 32;callbacks.pyline 34), andhasattr()will incorrectly returnTruefor missing keys instead ofFalse, which could cause subtle bugs in future code.💡 Optional improvement for correct Python semantics
def __getattr__(self, name): """Fallback to raw data for any missing attributes.""" - return self._raw_data.get(name) + if name in self._raw_data: + return self._raw_data[name] + raise AttributeError(f"'{type(self).__name__}' object has no attribute '{name}'")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/playbook-reference/kubernetes-examples/kubernetes-generic-example.rstsrc/robusta/core/playbooks/playbooks_event_handler_impl.pysrc/robusta/integrations/kubernetes/base_event.pysrc/robusta/integrations/kubernetes/base_triggers.py
🧰 Additional context used
🧬 Code graph analysis (2)
src/robusta/integrations/kubernetes/base_event.py (4)
src/robusta/core/reporting/base.py (2)
Finding(248-406)FindingSubject(223-245)src/robusta/core/reporting/consts.py (3)
FindingSource(24-38)FindingSubjectType(42-72)from_kind(54-72)src/robusta/integrations/prometheus/models.py (1)
get_subject(226-227)src/robusta/integrations/kubernetes/autogenerated/events.py (14)
get_subject(215-223)get_subject(394-402)get_subject(416-424)get_subject(451-459)get_subject(473-481)get_subject(508-516)get_subject(530-538)get_subject(565-573)get_subject(587-595)get_subject(622-630)get_subject(644-652)get_subject(679-687)get_subject(701-709)get_subject(736-744)
src/robusta/integrations/kubernetes/base_triggers.py (4)
src/robusta/core/playbooks/trigger.py (1)
get(26-27)src/robusta/integrations/kubernetes/autogenerated/models.py (1)
get_api_version(9-12)tests/test_change_filters.py (1)
event(36-43)src/robusta/integrations/kubernetes/autogenerated/events.py (1)
KubernetesAnyChangeEvent(242-367)
🪛 Flake8 (7.3.0)
src/robusta/integrations/kubernetes/base_event.py
[error] 59-59: do not use bare 'except'
(E722)
🪛 Ruff (0.14.11)
src/robusta/integrations/kubernetes/base_event.py
59-59: Do not use bare except
(E722)
🔇 Additional comments (5)
src/robusta/core/playbooks/playbooks_event_handler_impl.py (1)
53-67: LGTM!The explicit
execution_event = Nonein the exception handler correctly ensures that if an exception occurs afterbuild_execution_eventsucceeds (e.g., during thecheck_change_filterscall at line 57), the event is properly cleared before the next loop iteration. This prevents the potentially corrupted event from being processed downstream.src/robusta/integrations/kubernetes/base_event.py (1)
35-69: LGTM - well-structured subject extraction.The method properly handles both Hikaru objects and
GenericKubernetesObject, with appropriate fallbacks for missing metadata. The use ofgetattrwith defaults andor {}patterns provides defensive access to optional fields.src/robusta/integrations/kubernetes/base_triggers.py (3)
241-255: Good fallback to generic wrapper instead of raising exception.The change from raising
ModelNotFoundExceptionto returning aGenericKubernetesObjectenables graceful handling of unsupported resource kinds. The warning log provides visibility into when generic handling is used.
274-279: Good default toKubernetesAnyChangeEventfor unknown kinds.This ensures unknown kinds can still be processed through the event pipeline instead of being silently dropped, which aligns with the PR's goal of supporting generic objects.
315-324: Change filters are bypassed for generic objects.When
objis aGenericKubernetesObject, this code skips the diff-based change filtering entirely and always returnsTrue. This means all updates to generic resources will trigger playbooks regardless ofchange_filtersconfiguration. Consider documenting this limitation or logging a debug message when change filters are bypassed.Is this intentional behavior? If users configure
change_filtersfor a playbook targeting generic resources, they may expect filtering to apply. Consider adding a debug log:if isinstance(execution_event.obj, GenericKubernetesObject): + logging.debug( + f"Change filters not applied for GenericKubernetesObject kind={getattr(execution_event.obj, 'kind', 'Unknown')}" + ) obj_filtered = execution_event.obj old_obj_filtered = execution_event.old_obj
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| Generic resource tracking | ||
| =============== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RST underline should match or exceed title length.
The underline =============== (15 characters) is shorter than the title "Generic resource tracking" (25 characters). In reStructuredText, the underline should be at least as long as the title text.
📝 Suggested fix
Generic resource tracking
-===============
+=========================📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Generic resource tracking | |
| =============== | |
| Generic resource tracking | |
| ========================= |
🤖 Prompt for AI Agents
In `@docs/playbook-reference/kubernetes-examples/kubernetes-generic-example.rst`
around lines 1 - 2, The RST title underline is shorter than the title text
"Generic resource tracking"; update the underline row (the line currently
containing `===============`) so it matches or exceeds the title length (at
least 25 characters) by extending the sequence of `=` characters to equal the
title length, ensuring the underline and title use the same adornment character.
| subjects: | ||
| - kind: ServiceAccount | ||
| name: robusta-forwarder-service-account | ||
| namespace: default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded namespace may not match actual deployment.
The ServiceAccount namespace is hardcoded to default, but Robusta is typically installed in its own namespace (e.g., robusta). Consider noting that users should adjust this to match their Robusta installation namespace.
📝 Suggested fix
subjects:
- kind: ServiceAccount
name: robusta-forwarder-service-account
- namespace: default
+ namespace: robusta # Adjust to match your Robusta installation namespace📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| subjects: | |
| - kind: ServiceAccount | |
| name: robusta-forwarder-service-account | |
| namespace: default | |
| subjects: | |
| - kind: ServiceAccount | |
| name: robusta-forwarder-service-account | |
| namespace: robusta # Adjust to match your Robusta installation namespace |
🤖 Prompt for AI Agents
In `@docs/playbook-reference/kubernetes-examples/kubernetes-generic-example.rst`
around lines 44 - 47, The ServiceAccount entry hardcodes namespace: default
which may not match users' Robusta installation; update the documentation
snippet around the subjects -> kind: ServiceAccount / name:
robusta-forwarder-service-account to either remove the hardcoded namespace or
add a clear note and example showing the namespace should be set to the Robusta
namespace (e.g., robusta) or replaced by a placeholder (e.g.,
<ROBUSTA_NAMESPACE>) so users adjust it to their deployment.
| customPlaybooks: | ||
| - triggers: | ||
| - on_kubernetes_resource_operation: | ||
| namespace_prefix: "monitoring" | ||
| name_prefix: "grafana-" | ||
| actions: | ||
| - create_finding: # | ||
| title: "resource $name in namespace $namespace was modified" | ||
| aggregation_key: "resource_modified" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Playbook example filters don't align with HTTPRoute context.
The example uses namespace_prefix: "monitoring" and name_prefix: "grafana-", which doesn't relate to the HTTPRoute scenario described earlier. This could confuse users following the guide. Also, the trailing # comment on line 80 appears incomplete.
📝 Suggested fix
customPlaybooks:
- triggers:
- on_kubernetes_resource_operation:
- namespace_prefix: "monitoring"
- name_prefix: "grafana-"
+ # Adjust these filters to match your HTTPRoute resources
+ namespace_prefix: ""
+ name_prefix: ""
actions:
- - create_finding: #
+ - create_finding:
title: "resource $name in namespace $namespace was modified"
aggregation_key: "resource_modified"🤖 Prompt for AI Agents
In `@docs/playbook-reference/kubernetes-examples/kubernetes-generic-example.rst`
around lines 74 - 82, The example's Kubernetes filters don't match the HTTPRoute
scenario and include a stray "#" comment; update the customPlaybooks entry for
on_kubernetes_resource_operation so namespace_prefix and name_prefix reflect the
HTTPRoute example (e.g., set namespace_prefix to the HTTPRoute namespace used
earlier and name_prefix to a prefix like "httproute-" or the actual HTTPRoute
name prefix), ensure the create_finding action uses the intended title,
aggregation_key, etc., and remove the trailing "#" after create_finding to
eliminate the incomplete comment.
| try: | ||
| subject_type = FindingSubjectType.from_kind(kind) | ||
| except: | ||
| # Fallback for unknown kinds | ||
| subject_type = FindingSubjectType.TYPE_NONE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid bare except clause.
The bare except: catches all exceptions including KeyboardInterrupt and SystemExit, which can mask critical issues. Based on the from_kind implementation in the relevant snippets, it returns TYPE_NONE for unknown kinds and shouldn't raise exceptions. This try-except may be unnecessary, but if kept, catch a specific exception.
📝 Suggested fix
# Determine the subject type from the kind
- try:
- subject_type = FindingSubjectType.from_kind(kind)
- except:
- # Fallback for unknown kinds
- subject_type = FindingSubjectType.TYPE_NONE
+ subject_type = FindingSubjectType.from_kind(kind)If defensive handling is preferred:
- except:
+ except Exception:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| subject_type = FindingSubjectType.from_kind(kind) | |
| except: | |
| # Fallback for unknown kinds | |
| subject_type = FindingSubjectType.TYPE_NONE | |
| # Determine the subject type from the kind | |
| subject_type = FindingSubjectType.from_kind(kind) |
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 59-59: do not use bare 'except'
(E722)
🪛 Ruff (0.14.11)
59-59: Do not use bare except
(E722)
🤖 Prompt for AI Agents
In `@src/robusta/integrations/kubernetes/base_event.py` around lines 57 - 61,
Replace the bare except around FindingSubjectType.from_kind by calling it
directly (subject_type = FindingSubjectType.from_kind(kind)) since from_kind
already returns TYPE_NONE for unknown kinds; if you prefer defensive handling,
catch only specific exceptions like (ValueError, TypeError) and handle or log
them rather than using a bare except, ensuring subject_type falls back to
FindingSubjectType.TYPE_NONE.
Use kubewatch customresources to track objects beyond v1 and get generic alerts