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

[Defend Workflows] Convert filterQuery to kql #161806

Merged
merged 15 commits into from
Aug 23, 2023

Conversation

tomsonpl
Copy link
Contributor

@tomsonpl tomsonpl commented Jul 12, 2023

This PR solves: https://github.com/elastic/security-team/issues/6988

⚠️ THIS INTRODUCES A BREAKING CHANGE ⚠️

  • replace filterQuery with a kql query string

@tomsonpl tomsonpl added chore release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.10.0 labels Jul 12, 2023
@tomsonpl tomsonpl self-assigned this Jul 12, 2023
@tomsonpl tomsonpl marked this pull request as ready for review July 13, 2023 11:41
@tomsonpl tomsonpl requested review from a team as code owners July 13, 2023 11:41
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

@tomsonpl
Copy link
Contributor Author

Hey @marshallmain, could you take a look if this makes sense to you? I hope this resolves the issues you mentioned to @patrykkopycinski . Big thank you in advance 👍

@tomsonpl tomsonpl changed the title [WIP] Convert filterQuery to kql [Defend Workflows] Convert filterQuery to kql Jul 13, 2023
@tomsonpl tomsonpl requested review from patrykkopycinski and removed request for pzl July 14, 2023 15:04
Copy link
Contributor

@marshallmain marshallmain left a comment

Choose a reason for hiding this comment

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

Looks great! Verified that the search strategies that can query as internal user are now passing KQL through buildEsQuery rather than injecting filters directly 👍

Copy link
Contributor

@gergoabraham gergoabraham left a comment

Choose a reason for hiding this comment

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

looks great! 💯 added one small comment for avoiding some duplication, but that's all 🚀

filter = filter + ` AND ${kql}`;
}

const filterQuery = getQueryFilter({ filter });
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you think about pushing the responsibility of concatenating the filters into getQueryFilter() (e.g. getConcatenatedQueryFilter(...filters: string[])? it would eliminate the need for the duplicated logic above, and inside the function it can simply filter out the empty strings and .join(' AND ') the others

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That totally makes sense, thanks @gergoabraham 👍

# Conflicts:
#	x-pack/plugins/osquery/server/routes/live_query/find_live_query_route.ts
#	x-pack/plugins/osquery/server/routes/live_query/get_live_query_details_route.ts
#	x-pack/plugins/osquery/server/routes/live_query/get_live_query_results_route.ts
#	x-pack/plugins/osquery/server/search_strategy/osquery/index.ts
@tomsonpl tomsonpl added release_note:breaking Breaking Change v8.11.0 and removed release_note:skip Skip the PR/issue when compiling release notes v8.10.0 labels Aug 22, 2023
@tomsonpl tomsonpl marked this pull request as ready for review August 22, 2023 14:35
@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 23, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Defend Workflows Cypress Tests #1 / Endpoint Policy Response from Endpoint List page should display policy response with errors should display policy response with errors
  • [job] [logs] Defend Workflows Endpoint Cypress Tests #5 / Isolate command From cases should isolate and release host should isolate and release host

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
osquery 1.0MB 1.0MB +232.0B
securitySolution 15.7MB 15.7MB -20.0B
total +212.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
osquery 51.6KB 51.1KB -528.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @tomsonpl

@tomsonpl tomsonpl merged commit 9d909cd into elastic:main Aug 23, 2023
37 of 38 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 23, 2023
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 23, 2023
* main: (150 commits)
  Fixes unnecessary autocompletes on HTTP methods (elastic#163233)
  [Defend Workflows] Convert filterQuery to kql  (elastic#161806)
  [Fleet] copy `inactivity_timeout` when duplicating agent policy (elastic#164544)
  Fix 7.17 forward compatibility with 8.2+ (elastic#164274)
  [ML] Fixes dark mode in flyouts and modals (elastic#164399)
  [Defend Workflows]Changes to policy settings are not persistent until a refresh (elastic#164403)
  [Security Solution][Endpoint] Fixes kibana crash when going back to policy details page (elastic#164329)
  Prepare the Security domain HTTP APIs for Serverless (elastic#162087)
  skip failing test suite (elastic#160986)
  [Security Solution] Fix flaky Event Filters test (elastic#164473)
  [EDR workflows] Osquery serverless tests (elastic#163795)
  [Fleet] Only show agent dashboard links if there is more than one non-server agent and if the dashboards exist (elastic#164469)
  [Chrome UI] Fix background color in serverless (elastic#164419)
  [DOCS] Saved objects - resolve import errors API (elastic#162825)
  Remove 'Create Rule' button from Rule Group page (elastic#164167)
  [Security Solution] expandable flyout - fix infinite loop in correlations (elastic#163450)
  [Remote Clusters] Update copy about port help text (elastic#164442)
  [api-docs] 2023-08-23 Daily api_docs build (elastic#164524)
  [data views] Disable scripted fields in serverless environment (elastic#163228)
  [Reporting] Fix - show diagnostic only when image reporting is enabled (elastic#164336)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Breaking Change chore release_note:breaking Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants