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

[ES body removal] @elastic/security-detection-engine #204858

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

afharo
Copy link
Member

@afharo afharo commented Dec 19, 2024

Summary

in preparation for 9.0, we're trying to remove the deprecated body param in the ES client.

To make it easier to review, the changes have been split into multiple PRs, trying to group them per code owner as much as possible.

However, unfortunately, due to cross-dependencies, your team may be pinged more than once. Apologies for that additional noise.

What changes?

Nothing changes internally. The ES client already places everything where it should be when performing the request to ES (URL vs. query string vs. body params).

The main change is in the usage: when using the JS ES client, developers don't need to identify what goes in the body and what goes in the URL. All settings are provided at the root level. So, in summary, the change is:

const response = await client.search({
  index: 'test',
-  body: {
    query: {
      match_all: {}
    }
-  }
})

For this reason, enabling the "Hide whitespace changes" option when reviewing is recommended.

Some exceptions to this rule:

  • Bulk APIs replace the body array with operations array (direct replacement)
  • Index Put Settings API replace body array with settings (direct replacement)

@afharo afharo added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) labels Dec 19, 2024
@afharo afharo self-assigned this Dec 19, 2024
@afharo afharo marked this pull request as ready for review January 21, 2025 11:43
@afharo afharo requested a review from a team as a code owner January 21, 2025 11:43
@afharo afharo requested a review from nkhristinin January 21, 2025 11:43
@afharo afharo added the Team:Detection Engine Security Solution Detection Engine Area label Jan 21, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-engine (Team:Detection Engine)

@yctercero
Copy link
Contributor

Removing body caused issues for us in MKI. Can we be sure to run this change through our tests in MKI?

…-deprecated-body/elastic/security-detection-engine
@vitaliidm vitaliidm self-requested a review January 28, 2025 17:29
@afharo
Copy link
Member Author

afharo commented Jan 28, 2025

@yctercero, thanks for raising awareness! I've added the label ci:serverless-test-all to make sure that we test against MKI.

@elasticmachine
Copy link
Contributor

elasticmachine commented Jan 28, 2025

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #57 / Query rule execution logic API @ess @serverless @serverlessQA Query type rules preview logged requests should return requests property when enable_logged_requests set to true
  • [job] [logs] FTR Configs #74 / Query rule execution logic API @ess @serverless @serverlessQA Query type rules preview logged requests should return requests property when enable_logged_requests set to true
  • [job] [logs] FTR Configs #57 / Query rule execution logic API @ess @serverless @serverlessQA Query type rules preview logged requests should return requests property when enable_logged_requests set to true
  • [job] [logs] FTR Configs #74 / Query rule execution logic API @ess @serverless @serverlessQA Query type rules preview logged requests should return requests property when enable_logged_requests set to true
  • [job] [logs] FTR Configs #91 / Threshold rule execution logic API @ess @serverless @serverlessQA Threshold type rules preview logged requests should return requests property when enable_logged_requests set to true
  • [job] [logs] FTR Configs #103 / Threshold rule execution logic API @ess @serverless @serverlessQA Threshold type rules preview logged requests should return requests property when enable_logged_requests set to true
  • [job] [logs] FTR Configs #103 / Threshold rule execution logic API @ess @serverless @serverlessQA Threshold type rules preview logged requests should return requests property when enable_logged_requests set to true
  • [job] [logs] FTR Configs #91 / Threshold rule execution logic API @ess @serverless @serverlessQA Threshold type rules preview logged requests should return requests property when enable_logged_requests set to true

History

cc @afharo

…ion-engine' of https://github.com/afharo/kibana into esclient/remove-deprecated-body/elastic/security-detection-engine
@vitaliidm vitaliidm requested a review from a team as a code owner January 29, 2025 10:06
@vitaliidm
Copy link
Contributor

@nkhristinin
FYI: I added quite a lot changes to preview logged requests utils, so I removed myself from reviewers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) ci:build-serverless-image elasticsearch-js-9 release_note:skip Skip the PR/issue when compiling release notes Team:Detection Engine Security Solution Detection Engine Area
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants