Skip to content
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

Add 2 drilldowns #3146

Merged
merged 10 commits into from
Oct 17, 2024
Merged

Add 2 drilldowns #3146

merged 10 commits into from
Oct 17, 2024

Conversation

patel-bhavin
Copy link
Contributor

Adding two default drilldowns to all detections - TTP, Anomaly, TTP except experiemental/deprecated

@ljstella
Copy link
Contributor

Testing with the current main branch of contentctl produces the following validation errors:

File: detections/endpoint/detect_critical_alerts_from_security_tools.yml
Error: 1 validation error for Detection
  Value error, This detection is required to have 2 drilldown_searches, but only has [0] [type=value_error, input_value={'name': 'Detect Critical...rom_security_tools.yml'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/value_error

File: detections/endpoint/windows_modify_registry_disable_rdp.yml
Error: 1 validation error for Detection
  Value error, This detection is required to have 2 drilldown_searches, but only has [0] [type=value_error, input_value={'name': 'Windows Modify ...gistry_disable_rdp.yml'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/value_error

File: detections/endpoint/windows_modify_registry_on_smart_card_group_policy.yml
Error: 1 validation error for Detection
  Value error, This detection is required to have 2 drilldown_searches, but only has [0] [type=value_error, input_value={'name': 'Windows Modify ..._card_group_policy.yml'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/value_error

VALIDATION FAILED

I'm not sure why we're requiring two drilldown searches- that probably has to change, but in the meantime, these probably need to be updated.

Let's sync when you're online about some of the testing that was done with Correlations specifically, since UI elements in ES are driven by their drilldown searches, which require specifically crafted searches.

@pyth0n1c
Copy link
Collaborator

Testing with the current main branch of contentctl produces the following validation errors:

File: detections/endpoint/detect_critical_alerts_from_security_tools.yml
Error: 1 validation error for Detection
  Value error, This detection is required to have 2 drilldown_searches, but only has [0] [type=value_error, input_value={'name': 'Detect Critical...rom_security_tools.yml'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/value_error

File: detections/endpoint/windows_modify_registry_disable_rdp.yml
Error: 1 validation error for Detection
  Value error, This detection is required to have 2 drilldown_searches, but only has [0] [type=value_error, input_value={'name': 'Windows Modify ...gistry_disable_rdp.yml'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/value_error

File: detections/endpoint/windows_modify_registry_on_smart_card_group_policy.yml
Error: 1 validation error for Detection
  Value error, This detection is required to have 2 drilldown_searches, but only has [0] [type=value_error, input_value={'name': 'Windows Modify ..._card_group_policy.yml'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/value_error

VALIDATION FAILED

I'm not sure why we're requiring two drilldown searches- that probably has to change, but in the meantime, these probably need to be updated.

Let's sync when you're online about some of the testing that was done with Correlations specifically, since UI elements in ES are driven by their drilldown searches, which require specifically crafted searches.

The detections missing drilldowns have been updated - but this doesn't address your additional points.
As you noted, let's have an offline chat about this and update the PR with our conclusions.

@ljstella
Copy link
Contributor

ljstella commented Oct 16, 2024

Current status of appinspect job: Its now targeting this branch appropriately so builds are actually happening, and should work across the board a bit better. The failures on that job are real. However, they may have been unintended-

While hunting searches are excluded from the drilldown requirement, they are still getting a line added to their stanzas in savedsearches.conf that looks like this:

action.notable.param.drilldown_searches = []

This is because of the template changes made to add drilldown support. We can bump all those versions once, now, and keep this as a standard way we're going to output drilldowns, or we can go back to contentctl, change the template, try not to break anything, and not bump those versions.

Details about template:
This line is where we add them: https://github.com/splunk/contentctl/blob/cfda377c6887e28e02bb1798382ac0070b7983c2/contentctl/output/templates/savedsearches_detections.j2#L115

At first, I was a bit confused as to why it was there/what was going on. After loading it up in an IDE that identified pairs of brackets, I collapsed a few sections and found out the {{if}} block that it was inside was just the main one here at the top: https://github.com/splunk/contentctl/blob/cfda377c6887e28e02bb1798382ac0070b7983c2/contentctl/output/templates/savedsearches_detections.j2#L4

And that's how we got them on searches that don't require them.

Copy link
Contributor

@ljstella ljstella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fine to merge now.

:shipit: I guess

@patel-bhavin patel-bhavin merged commit f5cf688 into develop Oct 17, 2024
5 of 6 checks passed
@patel-bhavin patel-bhavin deleted the all_drilldowns branch October 17, 2024 16:07
@patel-bhavin patel-bhavin added this to the v4.43.0 milestone Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants