[Security Solution] [Elastic AI Assistant] Adds internal Get Evaluate API and migrates Post Evaluate API to OAS #176025

spong · 2024-01-31T22:28:17Z

Important

This PR is a reintroduction of #175338, which was reverted due to sporadic jest failures within the security_solution plugin. Root cause was identified and detailed in #176005 (comment).

Summary

In #174317 we added support for OpenAPI codegen, this PR builds on that functionality by migrating the Post Evaluate route /internal/elastic_assistant/evaluate to be backed by an OAS, and adds a basic Get Evaluate route for rounding out the enhancements outlined in https://github.com/elastic/security-team/issues/8167 (to be in a subsequent PR).

Changes include:

Migration of Post Evaluate route to OAS
Migration of Post Evaluate route to use versioned router
Extracted evaluate API calls from x-pack/packages/kbn-elastic-assistant/impl/assistant/api/api.tsx to x-pack/packages/kbn-elastic-assistant/impl/assistant/api/evaluate/evaluate.tsx
- Co-located relevant use_perform_evaluation hook
Adds Get Evaluate route, and corresponding use_evaluation_data hook. Currently only returns agentExecutors to be selected for evaluation.
API versioning constants added to x-pack/packages/kbn-elastic-assistant-common/impl/schemas/index.ts
Adds new buildRouteValidationWithZod function to x-pack/plugins/elastic_assistant/server/schemas/common.ts for validating routes against OAS generated zod schemas.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

…fix'

elasticmachine · 2024-01-31T22:28:20Z

Pinging @elastic/security-solution (Team: SecuritySolution)

spong · 2024-01-31T22:41:05Z

x-pack/plugins/security_solution/public/common/mock/mock_assistant_provider.tsx

@@ -22,7 +22,6 @@ window.HTMLElement.prototype.scrollIntoView = jest.fn();
 export const MockAssistantProviderComponent: React.FC<Props> = ({ children }) => {
  const actionTypeRegistry = actionTypeRegistryMock.create();
  const mockHttp = httpServiceMock.createStartContract({ basePath: '/test' });
-  mockHttp.get.mockResolvedValue([]);


This was the mock causing the 'async issue' which resulted in the revert discussed in #176005 (comment). Removing the mock to see what tests fail, and then will update the mocks accordingly.

And there's a 🟢 build with this mock removed. 🎉 Guess it wasn't needed? 😅 Let's verify and re-run the flakey test runner on the security solution tests.

Committed flakey-test runner changes (from #176005) in ef5b297 to ensure there really isn't an error on the security solution side. Will remove once test cycle is complete.

🟢 25/25, and that's a success! Here's the build link. Flakey-test runner changes reverted in c2fc97b. This is ready for re-review @stephmilovic.

cc @jbudz

Upon further testing locally, I was able to isolate the issue to this test here:

kibana/x-pack/plugins/security_solution/public/common/components/top_n/index.test.tsx

Lines 358 to 377 in 15ca37f

describe('rendering in alerts context', () => {

detectionAlertsTables.forEach((tableId) => {

test(`defaults to the 'Alert events' option when rendering in Alerts`, async () => {

const wrapper = mount(

<TestProviders store={store}>

<StatefulTopN

{...{

...testProps,

scopeId: tableId,

}}

/>

</TestProviders>

);

await waitFor(() => {

const props = wrapper.find('[data-test-subj="top-n"]').first().props() as Props;

expect(props.defaultView).toEqual('alert');

});

});

});

});

Fix to this test is provided in: 0b4028c

spong · 2024-02-01T04:25:16Z

x-pack/plugins/security_solution/public/common/components/top_n/index.test.tsx

@@ -372,6 +372,7 @@ describe('StatefulTopN', () => {
          const props = wrapper.find('[data-test-subj="top-n"]').first().props() as Props;
          expect(props.defaultView).toEqual('alert');
        });
+        wrapper.unmount();


FYI @elastic/security-threat-hunting-explore, this test here was a ⏲️ 💣 waiting for just the right conditions to 💥. I just so happened to stumble upon those conditions with this PR here: #175338 (comment)

Will run the flakey-test runner again with this change to make sure everything is 👍 .

Flakey Test runner added in cebd519

Tests results: 🟢 25/25, build link 🎉

Flakey test runner reverted in d44e4af

stephmilovic

LGTM, sorry jest was mean to you!

kibana-ci · 2024-02-01T15:53:37Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 60f250c

Failed CI Steps

Test Failures

[job] [logs] Investigations - Security Solution Cypress Tests #6 / Basic esql search and filter operations "before each" hook for "should remove the query when the back button is pressed after adding a query" "before each" hook for "should remove the query when the back button is pressed after adding a query"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	4940	4945	+5

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/elastic-assistant-common`	32	57	+25

Async chunks

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

id	before	after	diff
`securitySolution`	11.4MB	11.4MB	+3.0KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/elastic-assistant-common`	34	59	+25

History

💛 Build #190652 was flaky d44e4af
💔 Build #190650 failed cebd519
💚 Build #190634 succeeded c2fc97b
💔 Build #190631 failed ef5b297
💚 Build #190625 succeeded cc92572

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

cc @spong

… API and migrates Post Evaluate API to OAS (elastic#176025) > [!IMPORTANT] > This PR is a reintroduction of elastic#175338, which was [reverted](elastic#175338 (comment)) due to sporadic jest failures within the `security_solution` plugin. Root cause was identified and detailed in elastic#176005 (comment). ## Summary In elastic#174317 we added support for OpenAPI codegen, this PR builds on that functionality by migrating the `Post Evaluate` route `/internal/elastic_assistant/evaluate` to be backed by an OAS, and adds a basic `Get Evaluate` route for rounding out the enhancements outlined in elastic/security-team#8167 (to be in a subsequent PR). Changes include: * Migration of `Post Evaluate` route to OAS * Migration of `Post Evaluate` route to use versioned router * Extracted `evaluate` API calls from `x-pack/packages/kbn-elastic-assistant/impl/assistant/api/api.tsx` to `x-pack/packages/kbn-elastic-assistant/impl/assistant/api/evaluate/evaluate.tsx` * Co-located relevant `use_perform_evaluation` hook * Adds `Get Evaluate` route, and corresponding `use_evaluation_data` hook. Currently only returns `agentExecutors` to be selected for evaluation. * API versioning constants added to `x-pack/packages/kbn-elastic-assistant-common/impl/schemas/index.ts` * Adds new `buildRouteValidationWithZod` function to `x-pack/plugins/elastic_assistant/server/schemas/common.ts` for validating routes against OAS generated zod schemas. ### Checklist Delete any items that are not applicable to this PR. - [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [X] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <[email protected]>

spong and others added 7 commits January 23, 2024 10:45

Migrates post evaluate API to OAS and adds new get evaluate API

f12be81

Merge branch 'main' of github.com:elastic/kibana into eval+++

b686fd5

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

bb6a8f8

…fix'

Linter fix

bcce197

Merge branch 'main' into eval+++

6bd9426

Merge branch 'main' into eval+++

df1a836

Merge branch 'main' into eval+++

c4b569f

spong requested a review from stephmilovic January 31, 2024 22:28

spong self-assigned this Jan 31, 2024

spong requested a review from a team as a code owner January 31, 2024 22:28

Removes http.get mock override in MockAssistantProvider

8b29628

spong requested a review from a team as a code owner January 31, 2024 22:36

Merge branch 'main' of github.com:elastic/kibana into eval+++

cc92572

spong commented Jan 31, 2024

View reviewed changes

spong mentioned this pull request Jan 31, 2024

Jest flakiness testing #176005

Closed

Test SecuritySolution jest tests for flake

ef5b297

spong requested a review from a team as a code owner February 1, 2024 00:01

Revert flake test changes

c2fc97b

jbudz removed the request for review from a team February 1, 2024 00:59

Fixes problematic test

43b3d5b

spong requested a review from a team as a code owner February 1, 2024 04:01

Updates test fix

0b4028c

spong commented Feb 1, 2024

View reviewed changes

Bring back the flakey test runner

cebd519

And away with the flakey test runner one last time...

d44e4af

andrew-goldstein approved these changes Feb 1, 2024

View reviewed changes

Merge branch 'main' into eval+++

60f250c

stephmilovic approved these changes Feb 1, 2024

View reviewed changes

spong merged commit 859e440 into elastic:main Feb 1, 2024
38 checks passed

spong deleted the eval+++ branch February 1, 2024 17:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] [Elastic AI Assistant] Adds internal Get Evaluate API and migrates Post Evaluate API to OAS #176025

[Security Solution] [Elastic AI Assistant] Adds internal Get Evaluate API and migrates Post Evaluate API to OAS #176025

spong commented Jan 31, 2024

elasticmachine commented Jan 31, 2024

spong Jan 31, 2024 •

edited

Loading

spong Feb 1, 2024

spong Feb 1, 2024 •

edited

Loading

spong Feb 1, 2024 •

edited

Loading

spong Feb 1, 2024

spong Feb 1, 2024

stephmilovic left a comment

kibana-ci commented Feb 1, 2024

API count

	describe('rendering in alerts context', () => {
	detectionAlertsTables.forEach((tableId) => {
	test(`defaults to the 'Alert events' option when rendering in Alerts`, async () => {
	const wrapper = mount(
	<TestProviders store={store}>
	<StatefulTopN
	{...{
	...testProps,
	scopeId: tableId,
	}}
	/>
	</TestProviders>
	);
	await waitFor(() => {
	const props = wrapper.find('[data-test-subj="top-n"]').first().props() as Props;
	expect(props.defaultView).toEqual('alert');
	});
	});
	});
	});

[Security Solution] [Elastic AI Assistant] Adds internal Get Evaluate API and migrates Post Evaluate API to OAS #176025

[Security Solution] [Elastic AI Assistant] Adds internal Get Evaluate API and migrates Post Evaluate API to OAS #176025

Conversation

spong commented Jan 31, 2024

Summary

Checklist

elasticmachine commented Jan 31, 2024

spong Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

spong Feb 1, 2024

Choose a reason for hiding this comment

spong Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

spong Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

spong Feb 1, 2024

Choose a reason for hiding this comment

spong Feb 1, 2024

Choose a reason for hiding this comment

stephmilovic left a comment

Choose a reason for hiding this comment

kibana-ci commented Feb 1, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

API count

History

spong Jan 31, 2024 •

edited

Loading

spong Feb 1, 2024 •

edited

Loading

spong Feb 1, 2024 •

edited

Loading