[Bug] Strip Non-Public Fields Prior to Uploading Rules #2986
Conversation
Mikaayenson
added
bug
Mikaayenson
changed the title
| meta["original"] = dict(id=rule.id, **rule.contents.metadata.to_dict()) | ||
| payload["rule_id"] = str(uuid4()) | ||
| payload = downgrade(payload, target_version) | ||
| rule_dict = rule.contents.to_dict()["rule"] |
There was a problem hiding this comment.
Yup, exactly what we discussed. to_api_format() would result in a dict AFTER the transform method was called, thus being non-compatible with the frozen jsonschema. 🔥
| meta = rule_dict.pop("meta") | ||
| rule_contents = TOMLRuleContents.from_dict({"rule": rule_dict, "metadata": meta, | ||
| "transform": rule.contents.transform}) | ||
| payload = rule_contents.to_api_format() | ||
| payload = strip_non_public_fields(min_stack_version, payload) |
There was a problem hiding this comment.
🔥 - downgrade rule, which goes through jsonschema with the to_dict() format. Then pop the meta to remove it from being in the rule object. Passing contents to TOMLRuleContents and then calling to_api_format() to make it compatible with Kibana API. Then stripping the non-public fields as discussed. This looks NICE!
There was a problem hiding this comment.
Exactly. This approach seemed cleaner than passing the rule object and updating all the function definitions.
| NON_PUBLIC_FIELDS = { | ||
| "related_integrations": (Version.parse('8.3.0'), None), | ||
| "required_fields": (Version.parse('8.3.0'), None), | ||
| "setup": (Version.parse('8.3.0'), None) |
There was a problem hiding this comment.
We have this in rule.py as well. Will we eventually be moving everything to non_public_fields or are we keeping build time and non-public fields separate in terms of functionality they are referenced for?
There was a problem hiding this comment.
Good catch. Originally this was scoped just for 2931. Now that 2930 is in this as well, the overlap of BUILD_FIELD_VERSIONS seems very duplicative. The only caveat is that I just remembered that setup should probably not be added in the NON_PUBLIC_FIELDS object.
Even though they are the same today, the idea was that they may diverge in the future. It may be safer to keep them separate for now.
There was a problem hiding this comment.
I think it makes sense to keep them separate. While it is duplicative, it may be more difficult to separate them later should it be necessary. 👍
There was a problem hiding this comment.
🟩 PR test steps passed.
Testing output
> python -m detection_rules kibana --provider-name cloud-basic --kibana-url *** upload-rule -id a00681e3-9ed6-447c-ab2c-be648821c622 -r
█▀▀▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄ ▄ █▀▀▄ ▄ ▄ ▄ ▄▄▄ ▄▄▄
█ █ █▄▄ █ █▄▄ █ █ █ █ █ █▀▄ █ █▄▄▀ █ █ █ █▄▄ █▄▄
█▄▄▀ █▄▄ █ █▄▄ █▄▄ █ ▄█▄ █▄█ █ ▀▄█ █ ▀▄ █▄▄█ █▄▄ █▄▄ ▄▄█
kibana_user: eric.forte
kibana_password:
Successful uploads:
- 163cd46c-316a-469b-872c-6cbe26e9f107Manual review, LGTM 👍
Mikaayenson
deleted the
2931-bug-non-public-fields-not-stripped-on-future-builds
branch
(cherry picked from commit 3f9e7ac)
(cherry picked from commit 3f9e7ac)
(cherry picked from commit 3f9e7ac)
(cherry picked from commit 3f9e7ac)
(cherry picked from commit 3f9e7ac)
(cherry picked from commit 3f9e7ac)
(cherry picked from commit 3f9e7ac)
Issues
Resolves #2931
Resolves #2930
Summary
Some fields are not public using the API. This PR removes these fields based on stack schema versions (min and max) so that the fields aren't used when trying to upload the rule to a different stack version (see the issue for more details).
strip_non_public_fieldsmethod to init and NON_PUBLIC_FIELDS variable in definitionsdowngrade_contents_from_ruleto downgrade and validate theto_dictversion of the rule, then builds and new TOMLRuleContents object to obtain ato_api_formatpayload prior to uploading.Testing
Testing on Old Branches
The report mentioned this issue occurred when trying to use an old version of out rules (on an older stack) to a future stack.
In testing I switched to the 8.8 branch since my cloud stack is 8.9 and tried to upload.
Test Failure Prior to Code Changes
Passing
Testing on New Terms Rule
Test Failure Prior to Code Changes
Passing