-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solutions] Adds bsearch service to FTR e2e tests to reduce flake, boilerplate, and technique choices #116211
Merged
FrankHassanabad
merged 9 commits into
elastic:master
from
FrankHassanabad:add-bsearch-service
Oct 27, 2021
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
0a6f5a3
Adds a bsearch service with auto-retry capabilities to reduce flake a…
FrankHassanabad 95f6d59
Merge branch 'master' into add-bsearch-service
FrankHassanabad c32e03f
Updates return type
FrankHassanabad cbf2a54
Fix 1 type-o where I was not destructuring for the test
FrankHassanabad c142fc0
Merge branch 'master' into add-bsearch-service
kibanamachine 43123b2
Merge branch 'master' into add-bsearch-service
FrankHassanabad 8914db2
Merge branch 'add-bsearch-service' of github.com:FrankHassanabad/kiba…
FrankHassanabad 2021ba3
Merge branch 'master' into add-bsearch-service
FrankHassanabad 7bded87
Merge branch 'master' into add-bsearch-service
kibanamachine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
| * in compliance with, at your election, the Elastic License 2.0 or the Server | ||
| * Side Public License, v 1. | ||
| */ | ||
|
|
||
| import expect from '@kbn/expect'; | ||
| import request from 'superagent'; | ||
| import type SuperTest from 'supertest'; | ||
| import { IEsSearchResponse } from 'src/plugins/data/common'; | ||
| import { FtrProviderContext } from '../ftr_provider_context'; | ||
| import { RetryService } from './retry/retry'; | ||
|
|
||
| /** | ||
| * Function copied from here: | ||
| * test/api_integration/apis/search/bsearch.ts without the compress | ||
| * | ||
| * Splits the JSON lines from bsearch | ||
| */ | ||
| const parseBfetchResponse = (resp: request.Response): Array<Record<string, any>> => { | ||
| return resp.text | ||
| .trim() | ||
| .split('\n') | ||
| .map((item) => JSON.parse(item)); | ||
| }; | ||
|
|
||
| /** | ||
| * Function copied from here: | ||
| * x-pack/test/rule_registry/common/lib/authentication/spaces.ts | ||
| * @param spaceId The space id we want to utilize | ||
| */ | ||
| const getSpaceUrlPrefix = (spaceId?: string): string => { | ||
| return spaceId && spaceId !== 'default' ? `/s/${spaceId}` : ``; | ||
| }; | ||
|
|
||
| /** | ||
| * Options for the send method | ||
| */ | ||
| interface SendOptions { | ||
| supertest: SuperTest.SuperTest<SuperTest.Test>; | ||
| options: object; | ||
| strategy: string; | ||
| space?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Bsearch factory which will return a new bsearch capable service that can reduce flake | ||
| * on the CI systems when they are under pressure and bsearch returns an async search | ||
| * response or a sync response. | ||
| * | ||
| * @example | ||
| * const supertest = getService('supertest'); | ||
| * const bsearch = getService('bsearch'); | ||
| * const response = await bsearch.send<MyType>({ | ||
| * supertest, | ||
| * options: { | ||
| * defaultIndex: ['large_volume_dns_data'], | ||
| * } | ||
| * strategy: 'securitySolutionSearchStrategy', | ||
| * }); | ||
| * expect(response).eql({ ... your value ... }); | ||
| */ | ||
| export const BSearchFactory = (retry: RetryService) => ({ | ||
| /** Send method to send in your supertest, url, options, and strategy name */ | ||
| send: async <T extends IEsSearchResponse>({ | ||
| supertest, | ||
| options, | ||
| strategy, | ||
| space, | ||
| }: SendOptions): Promise<T> => { | ||
| const spaceUrl = getSpaceUrlPrefix(space); | ||
| const { body } = await retry.try(async () => { | ||
| return supertest | ||
| .post(`${spaceUrl}/internal/search/${strategy}`) | ||
| .set('kbn-xsrf', 'true') | ||
| .send(options) | ||
| .expect(200); | ||
| }); | ||
|
|
||
| if (body.isRunning) { | ||
| const result = await retry.try(async () => { | ||
| const resp = await supertest | ||
| .post(`${spaceUrl}/internal/bsearch`) | ||
| .set('kbn-xsrf', 'true') | ||
| .send({ | ||
| batch: [ | ||
| { | ||
| request: { | ||
| id: body.id, | ||
| ...options, | ||
| }, | ||
| options: { | ||
| strategy, | ||
| }, | ||
| }, | ||
| ], | ||
| }) | ||
| .expect(200); | ||
| const [parsedResponse] = parseBfetchResponse(resp); | ||
| expect(parsedResponse.result.isRunning).equal(false); | ||
| return parsedResponse.result; | ||
| }); | ||
| return result; | ||
| } else { | ||
| return body; | ||
| } | ||
| }, | ||
| }); | ||
|
|
||
| /** | ||
| * Bsearch provider which will return a new bsearch capable service that can reduce flake | ||
| * on the CI systems when they are under pressure and bsearch returns an async search response | ||
| * or a sync response. | ||
| */ | ||
| export function BSearchProvider({ | ||
| getService, | ||
| }: FtrProviderContext): ReturnType<typeof BSearchFactory> { | ||
| const retry = getService('retry'); | ||
| return BSearchFactory(retry); | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I haven't found other services using
expectinside of their functions. Not sure I see any issue with keeping it there but just wanted to see if there are other instances ofexpectused within FTR services.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.
It might be cool to provide a parameter where users of the bsearch service could specifiy what HTTP status code to expect.
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.
The expect errors will trigger the retries which is why they're here. I will add the HTTP status for people to expect if they have the need for other than 200, but I think at the moment we aren't concerned about testing bsearch results as we are just trying to ensure the endpoints all work.