[Bug] [DAC] Add filtering to export-rules-from-repo#5769
Conversation
Bug - GuidelinesThese guidelines serve as a reminder set of considerations when addressing a bug in the code. Documentation and Context
Code Standards and Practices
Testing
Additional Checks
|
| # Get exceptions in API format (only those linked to the exported rules) | ||
| if include_exceptions: | ||
| exceptions = [d.contents.to_api_format() for d in cl.items if isinstance(d.contents, TOMLExceptionContents)] | ||
| exceptions = [ |
There was a problem hiding this comment.
Since we repeat for TOMLActionConnectorContents, should we dedicate a method and yield results? Not sure if we expect to do this for other TOML... related types.
There was a problem hiding this comment.
This is a good idea, and probably something we should do, but I think that should be part of a more through re-work of the object handling, given some of the nuance of the different TOML... related types. Example: #5181 (comment)
…and-includes-all-exceptionsaction-connectors
Mikaayenson
left a comment
There was a problem hiding this comment.
IINM, anyone who previously used import-rules-into-repo with Kibana API exports will have exception/action connector TOML files with Kibana internal ids in metadata.rule_ids instead of rule_id UUIDs. Those items won't match in scoped exports until re-imported. Worth a note in release docs.
| GenericCollectionContentTypes = TOMLActionContents | TOMLActionConnectorContents | TOMLExceptionContents | ||
|
|
||
|
|
||
| def matches_rule_ids(item: GenericCollectionTypes, rule_ids: set[str]) -> bool: |
There was a problem hiding this comment.
nit: This won't work on TomlAction since the Action has rule_id vs rule_ids. More of a type hinting issue atm but could cause an opaque bug in the future.
| return callback | ||
|
|
||
|
|
||
| class GenericCollection: |
There was a problem hiding this comment.
nit: with the isinstance(...) and matches_rule_ids(...) pattern repeats 4 times across main.py and kbwrap.py. A small helper on GenericCollection would centralize it.
| exception_list_rule_table[exception_id].append({"id": contents["id"], "name": contents["name"]}) | ||
| exception_list_rule_table[exception_id].append( | ||
| { | ||
| "id": contents.get("rule_id") or contents.get("id"), |
There was a problem hiding this comment.
nit: This may be confusing interchanging id and rule_id and later from_exceptions_dict, rule["id"]. It's a bit cosmetic so we should over index, but its worth a small comment in the code.
Pull Request
Issue link(s):
Resolves #5768
Summary - What I changed
The
export-rules-from-repocommand with--include-exceptions(-e) or--include-action-connectors(-ac) was adding all exception lists and all action connectors from the repo into the exported NDJSON, instead of only those linked to the rules being exported.kibana import-rulesalready scopes correctly via_matches_rule_ids; export now does the same.Changes:
detection_rules/generic_loader.pymatches_rule_ids(item, rule_ids)that returns whether an exception or action-connector item’smetadata.rule_idsoverlaps the given set. Usesgetattr(item.contents.metadata, "rule_ids", [])so it works with existing metadata types.detection_rules/main.py_export_rules():matches_rule_idsfrom.generic_loader.rule_ids = {r.id for r in rules}.isinstance(d.contents, TOMLExceptionContents) and matches_rule_ids(d, rule_ids).isinstance(d.contents, TOMLActionConnectorContents) and matches_rule_ids(d, rule_ids).detection_rules/kbwrap.pymatches_rule_idsfrom.generic_loader(removedGenericCollectionTypesfrom import)._matches_rule_idsand use the sharedmatches_rule_ids(d, rule_ids)in both the exception and action-connector list comprehensions forkibana import-rules.Result: Subset exports (e.g.
--rule-id X -e -o out.ndjson) only include exception lists and action connectors whosemetadata.rule_idsintersect the exported rules. Full-repo export still includes all exceptions/connectors that are linked to any exported rule (unchanged in practice when exporting all rules).How To Test
Export a single rule with exceptions (repo must have at least one rule that references an exception list):
/tmp/single.ndjson: it should contain one rule object plus only exception list object(s) whosemetadata.rule_ids(or equivalent in API format) include<rule-id>, not every exception list in the repo.Export with action connectors (if repo has rules that reference action connectors):
kibana import-rules unchanged: Run existing flows that use
kibana import-rules(e.g. with-e/-ac) to confirm behavior is unchanged; they now call the sharedmatches_rule_idsinstead of the removed local helper.Checklist
bug,enhancement,schema,maintenance,Rule: New,Rule: Deprecation,Rule: Tuning,Hunt: New, orHunt: Tuningso guidelines can be generatedmeta:rapid-mergelabel if planning to merge within 24 hoursContributor checklist