feat(exporters): add eventFieldFilter to HTTP exporter#802
Open
RohanKaran wants to merge 5 commits intokubescape:mainfrom
Open
feat(exporters): add eventFieldFilter to HTTP exporter#802RohanKaran wants to merge 5 commits intokubescape:mainfrom
RohanKaran wants to merge 5 commits intokubescape:mainfrom
Conversation
Add EventFieldFilterConfig with allowList and denyList support to HTTPExporterConfig. The filter is applied as a JSON post-processing step inside sendHTTPRequest before the payload is sent. - allowList keeps only the specified fields; denyList removes them - both support dot notation for nested fields (e.g. spec.processTree) and fields inside slices (e.g. spec.alerts.severity) - allowList takes precedence over denyList when both are set - large integers (PIDs, nanosecond timestamps) are preserved via json.Decoder.UseNumber() - filter is HTTP-exporter-specific; stdout and other exporters are unaffected Signed-off-by: rohankaran <rohankaran001@gmail.com>
…tering, and rate limits - change headers field from map to list of key-value pairs - add support for path, queryParams, maxAlertsPerMinute, and eventFieldFilter - introduce eventFieldFilter with allowList and denyList for payload field control - update documentation with examples to reduce payload size or restrict sent fields Signed-off-by: rohankaran <rohankaran001@gmail.com>
…ieldFilter scope note - Remove '/v1/runtimealerts' suffix from example URLs for clarity - Add note clarifying eventFieldFilter applies to all payloads sent through the exporter Signed-off-by: rohankaran <rohankaran001@gmail.com>
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds an EventFieldFilter (configurable allowList or denyList with dot-notation) that prunes or removes fields from JSON alert payloads; integrates the filter into the HTTP exporter pipeline; adds unit tests; and updates configuration documentation and examples. ChangesEvent Field Filtering for HTTP Exports
Sequence DiagramsequenceDiagram
participant Client as HTTP Exporter
participant Filter as Field Filter
participant Sender as HTTP Sender
Client->>Client: Marshal event to JSON
Client->>Filter: FilterJSON(jsonPayload)
alt Has Field Filter Config
Filter->>Filter: Parse JSON to map
Filter->>Filter: Apply AllowList OR DenyList
Filter->>Filter: Recursively filter nested fields and slices
Filter->>Client: Return filtered JSON bytes
else No Filter Config
Filter->>Client: Return original JSON bytes
end
Client->>Sender: Send HTTP request with payload
Sender->>Sender: Apply headers, retries, bulking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/exporters/field_filter.go (1)
50-72: 💤 Low value
FilterJSONre-marshals withjson.Marshal, which applies HTML escaping.
json.Marshalescapes<,>, and&in string values to their Unicode escape sequences (e.g.,<→\u003c). If alert messages or field values contain these characters, the outgoing payload will differ from what a raw marshal of the original struct would produce. Downstream consumers relying on exact byte content (e.g., signature validation) may see unexpected output.If this is a concern, swap
json.Marshalforjson.NewEncoderwithSetEscapeHTML(false):🛠️ Proposed fix
+import "bytes" -return json.Marshal(m) +var buf bytes.Buffer +enc := json.NewEncoder(&buf) +enc.SetEscapeHTML(false) +if err := enc.Encode(m); err != nil { + return nil, err +} +return bytes.TrimRight(buf.Bytes(), "\n"), nil🤖 Prompt for 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. In `@pkg/exporters/field_filter.go` around lines 50 - 72, EventFieldFilter.FilterJSON currently returns json.Marshal(m) which applies HTML escaping; replace that marshal with a json.Encoder that disables HTML escaping: create a bytes.Buffer, use enc := json.NewEncoder(&buf); enc.SetEscapeHTML(false); encode m with enc.Encode(m) and return the buffer bytes (trim the trailing newline Encode adds) and nil error; keep the existing logic using f.useAllow, f.allowSet, f.denySet, applyAllowList and removePath unchanged.pkg/exporters/field_filter_test.go (1)
210-250: 💤 Low valueTest coverage looks comprehensive; add a case for slice items with no matching allowed fields.
The existing
TestFilterJSON_AllowListSliceFieldcovers the happy path (all items have the allowed field). Consider adding a case where a slice item has none of the allowed fields to verify the current behavior (empty{}is preserved) and make it explicit — especially if the fix from theapplyAllowListcomment above is applied (empty items would then be omitted).Also consider a test for
FilterJSONreturning an error when given invalid JSON bytes, to exercise that error path.🤖 Prompt for 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. In `@pkg/exporters/field_filter_test.go` around lines 210 - 250, Add a new unit case to TestFilterJSON_AllowListSliceField (or create TestFilterJSON_AllowListSliceField_EmptyItem) that includes at least one alerts slice item lacking any allowed fields (e.g., an alert with only "ruleID" and "severity" removed by AllowList ["kind","spec.alerts.message"]) and assert the current expected behavior: either an empty map{} is preserved in the output alerts slice (require.Len/require.NotNil and assert.Empty for that item) or, if you intend to change behavior via applyAllowList, assert that the item is omitted (shorter alerts slice), updating expectations accordingly; additionally add a small test (e.g., TestFilterJSON_InvalidJSON) that calls EventFieldFilter.FilterJSON with invalid JSON bytes and asserts an error is returned to cover the error path.
🤖 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 `@pkg/exporters/field_filter.go`:
- Around line 100-110: applyAllowList currently retains empty maps in slices:
when an item is a map[string]any and applyAllowList returns an empty map (no
allowed sub-fields matched), that empty {} is kept in newSlice; change the logic
in applyAllowList (the slice-handling branch in pkg/exporters/field_filter.go)
to skip adding map items that become empty (len(resultingMap)==0) so they are
omitted from newSlice, while still preserving non-map items unchanged and
keeping index-safe behavior (i.e., append only non-empty maps or original
non-map items).
---
Nitpick comments:
In `@pkg/exporters/field_filter_test.go`:
- Around line 210-250: Add a new unit case to TestFilterJSON_AllowListSliceField
(or create TestFilterJSON_AllowListSliceField_EmptyItem) that includes at least
one alerts slice item lacking any allowed fields (e.g., an alert with only
"ruleID" and "severity" removed by AllowList ["kind","spec.alerts.message"]) and
assert the current expected behavior: either an empty map{} is preserved in the
output alerts slice (require.Len/require.NotNil and assert.Empty for that item)
or, if you intend to change behavior via applyAllowList, assert that the item is
omitted (shorter alerts slice), updating expectations accordingly; additionally
add a small test (e.g., TestFilterJSON_InvalidJSON) that calls
EventFieldFilter.FilterJSON with invalid JSON bytes and asserts an error is
returned to cover the error path.
In `@pkg/exporters/field_filter.go`:
- Around line 50-72: EventFieldFilter.FilterJSON currently returns
json.Marshal(m) which applies HTML escaping; replace that marshal with a
json.Encoder that disables HTML escaping: create a bytes.Buffer, use enc :=
json.NewEncoder(&buf); enc.SetEscapeHTML(false); encode m with enc.Encode(m) and
return the buffer bytes (trim the trailing newline Encode adds) and nil error;
keep the existing logic using f.useAllow, f.allowSet, f.denySet, applyAllowList
and removePath 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: df8b227f-5444-43b7-9b5f-033591c36596
📒 Files selected for processing (4)
docs/CONFIGURATION.mdpkg/exporters/field_filter.gopkg/exporters/field_filter_test.gopkg/exporters/http_exporter.go
- Replace json.Marshal with json.NewEncoder + SetEscapeHTML(false) to
prevent < > & in alert messages being mangled to \u003c etc.
- applyAllowList now omits map items from slices that become empty after
filtering, instead of keeping phantom {} objects
- Add TestFilterJSON_InvalidJSON_ReturnsError, TestFilterJSON_AllowListSliceField_OmitsEmptyItems,
TestFilterJSON_NoHTMLEscaping
Signed-off-by: rohankaran <rohankaran001@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/exporters/field_filter_test.go (2)
127-127: 💤 Low valueInconsistent use of
[]interface{}vs[]any.Line 127 uses
[]interface{}{"alert1"}while all other tests use[]any{...}. In Go 1.18+anyis an alias forinterface{}, but the file should be consistent.🛠️ Proposed fix
- "alerts": []interface{}{"alert1"}, + "alerts": []any{"alert1"},🤖 Prompt for 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. In `@pkg/exporters/field_filter_test.go` at line 127, The test has an inconsistent collection type: change the value for the "alerts" key currently written as []interface{}{"alert1"} to use the alias []any{"alert1"} so it matches the rest of the file; locate the occurrence in the test data map within field_filter_test (the map literal containing "alerts") and replace the type to maintain consistent use of any across the file.
50-50: ⚡ Quick winIgnored
json.Marshalerrors in test setup may mask test construction bugs.All test functions ignore the error from
json.Marshalwith_. Whilejson.Marshalrarely fails formap[string]anywith plain values, silently discarding the error means a malformeddatabyte slice (e.g.,nil) could flow intoFilterJSONand produce a misleading test failure message. Preferrequire.NoErrorto fail fast at the setup step.🛠️ Proposed fix (representative example from line 50)
-data, _ := json.Marshal(input) +data, err := json.Marshal(input) +require.NoError(t, err)Also applies to: 78-78, 104-104, 131-131, 157-157, 187-187, 226-226, 280-280, 308-308, 334-334, 361-361
🤖 Prompt for 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. In `@pkg/exporters/field_filter_test.go` at line 50, Replace all ignored json.Marshal errors in the test setup (instances like data, _ := json.Marshal(input)) with explicit error checks: capture the error (data, err := json.Marshal(input)) and call require.NoError(t, err) immediately after so the test fails fast on marshal failures; apply this change for every occurrence in the field_filter_test.go file where json.Marshal produces data for calls to FilterJSON and other test setups.
🤖 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 `@pkg/exporters/field_filter_test.go`:
- Around line 146-169: The test TestFilterJSON_AllowListTakesPrecedence
currently can't prove deny list is ignored because it denies "ruleID" which is
already not allowed; update the test input so at least one field appears in both
AllowList and DenyList (e.g., set AllowList: []string{"message","ruleID"} and
DenyList: []string{"ruleID"}) and include "ruleID" in the marshaled input, then
call f.FilterJSON and assert that output["ruleID"] is present/equals the
original value (i.e., kept) while other non-allowed fields (like "extra") are
nil; keep references to EventFieldFilterConfig, AllowList, DenyList,
TestFilterJSON_AllowListTakesPrecedence, and FilterJSON when making the change.
---
Nitpick comments:
In `@pkg/exporters/field_filter_test.go`:
- Line 127: The test has an inconsistent collection type: change the value for
the "alerts" key currently written as []interface{}{"alert1"} to use the alias
[]any{"alert1"} so it matches the rest of the file; locate the occurrence in the
test data map within field_filter_test (the map literal containing "alerts") and
replace the type to maintain consistent use of any across the file.
- Line 50: Replace all ignored json.Marshal errors in the test setup (instances
like data, _ := json.Marshal(input)) with explicit error checks: capture the
error (data, err := json.Marshal(input)) and call require.NoError(t, err)
immediately after so the test fails fast on marshal failures; apply this change
for every occurrence in the field_filter_test.go file where json.Marshal
produces data for calls to FilterJSON and other test setups.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d43b721-aa73-4f72-ac9e-a6f16052805b
📒 Files selected for processing (2)
pkg/exporters/field_filter.gopkg/exporters/field_filter_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/exporters/field_filter.go
- Keep scalar leaf values (string, number, bool) when applying allow list filter - Add tests to verify scalar values are retained inside nested maps and slices - Strengthen TestFilterJSON_AllowListTakesPrecedence by including a field in both allow and deny lists to actually prove precedence Signed-off-by: rohankaran <rohankaran001@gmail.com>
RohanKaran
force-pushed
the
feature/event-field-filter
branch
from
6159953 to
85ac441
Compare
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
2 participants
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.
Closes #488
Adds
eventFieldFiltertoHTTPExporterConfigwithallowListanddenyListsupport. Both accept dot-notation paths (e.g.spec.processTree,spec.alerts.severity) including fields nested inside slices.allowListtakes precedence when both are set.Initially proposed filtering at
ExporterBuslevel, but each exporter serialises differently (HTTP uses JSON paths, stdout uses log field names), so a shared filter would silently no-op for one or the other. Scoped it toHTTPExporterConfiginstead, applied as a JSON post-processing step insendHTTPRequest.Summary by CodeRabbit
New Features
Documentation
Tests