-
Notifications
You must be signed in to change notification settings - Fork 611
Added logic to main.py to use the created_at and updated_at values if they exist #5444
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: main
Are you sure you want to change the base?
Conversation
…om the ndjson file if they exist.
|
👋 Thanks for the PR, unless I am mistaken, generally speaking when this code was implemented the decision was made that created and updated dates are specific to the toml file rather than the rule file from Kibana. I am certainly open to this change, but as the code sits currently, this is intended behavior. Given that I expect the outcome will be that we will want to include the functionality from the PR, we can continue to review and merge when we are satisfied, just wanting to note that this will be a change (or alternative) to the original import pipeline 👍 |
detection_rules/main.py
Outdated
| contents["author"] = [contents["author"]] | ||
|
|
||
| # Parse created_at and updated_at to creation_date and updated_date if they exist in contents | ||
| if "created_at" in contents: |
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.
Instead of using these try, except blocks, it would be better to match the style/formatting we use in this function by using .get(). The addition is smaller and does not need to add additional exception handling.
It could look something like this instead for this PR:
# Parse created_at and updated_at to creation_date and updated_date if they exist in contents
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
contents["creation_date"] = datetime.strptime(
contents.get("created_at", now), "%Y-%m-%dT%H:%M:%S.%fZ"
).strftime("%Y/%m/%d")
contents["updated_date"] = datetime.strptime(contents.get("updated_at", now), "%Y-%m-%dT%H:%M:%S.%fZ").strftime(
"%Y/%m/%d"
)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.
Thanks for the help, I just updated the functions to use the .get() function using your code.
@eric-forte-elastic do you think this is a big enough change to justify wrapping this code in a new commandline option to allow users to enable it if they want it? |
Introduce a new option `--dates-import` to parse `created_at` and `updated_at` fields from rule content. This allows users to import date metadata while preventing conflicts with existing date options.
|
@eric-forte-elastic I updated the function to wrap it inside of a new --dates-import commandline option. This will provide this functionality without breaking existing use cases. |
This looks great! Now you would just have to bump the pyptoject toml version from |
This update increments the version number in the project metadata to reflect the upcoming release. No other changes were made.
|
@eric-forte-elastic I updated the pyproject.toml version number, but It looks like I don't have permissions to update the issue labels in this repo. |
Enhancement - GuidelinesThese guidelines serve as a reminder set of considerations when addressing adding a feature to the code. Documentation and Context
Code Standards and Practices
Testing
Additional Checks
|
Roger, updated the issue with the labels. Looks like just linting now. |
Modified the handling of creation and updated dates to ensure that the datetime objects are timezone-aware by replacing the timezone info with UTC. This change improves the accuracy of date metadata in the rules.
|
I updated the code to fix the linting errors about timezone support. I tested the code again and it still works as intended on my system. |
Summary - What I changed
During testing I found that for my custom rules when exported from a cluster for the first time the
creation_dateandupdated_datevalues were being set to todays date instead of using thecreated_atandupdated_atvalues from the detection rules.This update adds logic to main.py (line 250) within the
import_rules_into_repofunction to use the values from the contents of the rule if those fields exist. This fixes the issue and now when rules are imported for the first time they retain the original values from the cluster.This logic will only be called when the
--dates-import | -dicommandline option is called with theimport_rules_into_repofunction.How To Test
import existing custom detection rules from a cluster to a repo where the
created_atandupdated_atdates are prior to today. Verify that thecreation_dateandupdated_dateuse the correct date from the rule file and are not set to today.Checklist
bug,enhancement,schema,maintenance,Rule: New,Rule: Deprecation,Rule: Tuning,Hunt: New, orHunt: Tuningso guidelines can be generatedContributor checklist