[Security Solution] Quickstart script tooling for Detections and Response #190634

marshallmain · 2024-08-15T18:29:17Z

Summary

Creates CLI script tooling for building data, rules, exceptions, and lists in any (local, cloud, serverless) environment for manual testing. The initial commits here add generated clients for accessing security solution, exceptions, and lists APIs and a placeholder script where those clients are set up for use. See README for more details.

Much of the code in this PR is auto-generated clients. The hand written code is intended to be primarily in quickstart/modules/, where we can add wrapper code to simplify the process for common test environment setup. For example, createValueListException takes an array of items and some metadata and automatically creates a new value list and an exception that references that value list. /modules/data/ contains functions to generate documents of arbitrary size, and we can add more functions to create various other types of documents.

…fix'

jbudz

packages/kbn-test/src/kbn_client/kbn_client_requester.ts

…-fix'

pheyos

kbn-test changes LGTM

marshallmain · 2024-08-26T14:35:32Z

packages/kbn-securitysolution-utils/src/client_concurrency/index.ts

+/**
+ * Helper function to call an API multiple times concurrently, basically like a client-side bulk API.
+ * 
+ * const responses = await clientConcurrency(
+        detectionsClient.createRule.bind(detectionsClient),
+        ruleCopies.map((rule) => ({
+          body: rule,
+        }))
+      );
+ * Note the `.bind` is crucial in `detectionsClient.createRule.bind(detectionsClient)` - without binding, `createRule` is called without
+      access to the detectionsClient object and it will fail at runtime - even though the code typechecks fine.
+ * @param clientFunction The API function to call multiple times
+ * @param inputs An array of input values. Each value in the array is passed to a separate invocation of the clientFunction.
+ * @param concurrency Number of simultaneous in-progress API calls that are allowed.
+ * @returns An array of the responses from the API calls.
+ */
+export const clientConcurrency = async <Input, Output>(
+  clientFunction: (input: Input) => Promise<Output>,
+  inputs: Input[],
+  concurrency: number = 10
+) => {
+  const limiter = limit(concurrency);
+  const promises = inputs.map((input) => limiter(() => clientFunction(input)));
+  return Promise.all(promises);
+};


This function is an alternative way of implementing concurrency compared to the one below (concurrentlyExec). IMO concurrentlyExec is a bit better because it's easy to forget the .bind in this implementation, but concurrentlyExec does require creating a wrapper function for each request which makes the code look a bit messier (but the compiler will help you out, unlike with .bind where the compiler does not help you).

I'll remove one of the concurrency implementations depending on which one reviewers think is better DX-wise.

IMHO concurrentlyExec has much more straightforward API. It's quite easy to forget adding .bind(). Though I think params should be renamed for better clarity. In fact concurrentlyExec accepts factories (factory is a well known and convenient pattern) so it should be reflected clearly like in the example below

type ActionFactory = () => Promise<Output>; export const concurrentlyExec = async <Output>( actionFactories: ActionFactor[], concurrency: number = 10 ) => { // ... }

banderror · 2024-08-26T15:00:27Z

Added the whole @elastic/security-detection-rule-management team to the list of reviewers so people get reminded about the PR over slack.

maximpn

@marshallmain thank you for extending code generation with quickstart scripts 🙏

I have concerns regarding generation context typing since it could decrease code readability. Technically sources and imports are almost the same and could be reused. As minimum it's better to get rid of ad hoc types.

Feel free reach me to discuss it in more details if something is unclear.

maximpn · 2024-08-27T09:46:53Z

packages/kbn-openapi-generator/src/template_service/templates/api_client_quickstart.handlebars

+  {{operationId}}RequestBodyInput,
+  {{operationId}}Response,
+{{/each}}
+} from '{{replace sourcePath 'schema.yaml' 'gen'}}';


It's better to make such processing when context is constructed. It will help to simplify the template. There is already an utility function getGeneratedFilePath() for that purpose.

maximpn · 2024-08-27T09:59:40Z

packages/kbn-openapi-generator/src/template_service/template_service.ts

+  compileTemplate: (
+    templateName: TemplateName,
+    context: GenerationContext & {
+      sources: Array<{ sourcePath: string; generationContext: GenerationContext }>;


Ad hoc typing only adds confusion here. context is a GenerationContext and also has sources with generationContext. Anyone reading this later on will need to spend time to inspect the upstream code to see what it actually means. I'd recommend to extend GenerationContext with sources.

If going deeper we already have imports field in GenerationContext. It looks like it could be reused after adjustments like checking generationContext and filling importing symbols.

My intent here was to avoid making GenerationContext into a recursive type, but yeah it was too confusing to have the context + generationContext properties and type mixing. I replaced this bit by separating the bundle generation context from the single file generation context and also creating a compileBundleTemplate function separate from compileTemplate - lmk if it looks better to you now. We had the imports field in the generation context for the bundles, but it didn't actually contain any data so it wasn't usable.

My idea regarding imports is that it's quite simple. Context could have necessary import entries when constructed. Looking at zod_operation_schema.handlebars using imports we can see that this approach will add imports for any import entry presented in imports.

{{#each imports}} import { {{#each this}}{{.}},{{/each}} } from "{{@key}}" {{/each}}

There is no obstacle to add there necessary entries like ${operationId}RequestQueryInputand ${operationId}}RequestParamsInput`. Of course it'll require reworking generation logic a big to support multiple contexts.

I'd investigate this option but this is just a simplification idea and isn't a blocker for now.

I think compileBundleTemplate is much clearer 👍

…fix'

maximpn

@marshallmain thanks for addressing my comments 👍

The only concern I have is double sorting in openapi_generator.ts.

maximpn · 2024-09-02T09:52:49Z

packages/kbn-openapi-generator/src/openapi_generator.ts

@@ -68,6 +69,10 @@ export const generate = async (config: GeneratorConfig) => {
    ({ generationContext }) =>
      generationContext.operations.length > 0 || generationContext.components !== undefined
  );
+  parsedSources.sort((a, b) => a.sourcePath.localeCompare(b.sourcePath));


Does it make sense to sort by sourcePath here while contexts are sorted by operationId on line 98?

I think so, we could sort the operations within each source first then sort the sources by the first operationId in each source but I thought sourcePath felt like a more natural way to sort the sources. The sources and operations are closely related (and often 1-to-1) but I think it makes sense to keep some differentiation between them and treat the sourcePath as a unique identifier for sources and operationId as a unique identifier for operations.

maximpn · 2024-09-02T10:05:55Z

packages/kbn-securitysolution-utils/src/client_concurrency/index.ts

+/**
+ * Helper function to call an API multiple times concurrently, basically like a client-side bulk API.
+ * 
+ * const responses = await clientConcurrency(
+        detectionsClient.createRule.bind(detectionsClient),
+        ruleCopies.map((rule) => ({
+          body: rule,
+        }))
+      );
+ * Note the `.bind` is crucial in `detectionsClient.createRule.bind(detectionsClient)` - without binding, `createRule` is called without
+      access to the detectionsClient object and it will fail at runtime - even though the code typechecks fine.
+ * @param clientFunction The API function to call multiple times
+ * @param inputs An array of input values. Each value in the array is passed to a separate invocation of the clientFunction.
+ * @param concurrency Number of simultaneous in-progress API calls that are allowed.
+ * @returns An array of the responses from the API calls.
+ */
+export const clientConcurrency = async <Input, Output>(
+  clientFunction: (input: Input) => Promise<Output>,
+  inputs: Input[],
+  concurrency: number = 10
+) => {
+  const limiter = limit(concurrency);
+  const promises = inputs.map((input) => limiter(() => clientFunction(input)));
+  return Promise.all(promises);
+};


IMHO concurrentlyExec has much more straightforward API. It's quite easy to forget adding .bind(). Though I think params should be renamed for better clarity. In fact concurrentlyExec accepts factories (factory is a well known and convenient pattern) so it should be reflected clearly like in the example below

type ActionFactory = () => Promise<Output>; export const concurrentlyExec = async <Output>( actionFactories: ActionFactor[], concurrency: number = 10 ) => { // ... }

maximpn · 2024-09-02T10:07:02Z

packages/kbn-openapi-generator/src/template_service/template_service.ts

+  compileTemplate: (
+    templateName: TemplateName,
+    context: GenerationContext & {
+      sources: Array<{ sourcePath: string; generationContext: GenerationContext }>;


I think compileBundleTemplate is much clearer 👍

…un variable name

kibana-ci · 2024-09-05T19:10:24Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: bd10ce0

Failed CI Steps

FTR Configs #5

Test Failures

[job] [logs] FTR Configs #5 / Saved Objects Management saved objects management with hidden types Delete modal should not delete the hidden objects when performing the operation

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lists`	289	340	+51
`securitySolution`	5675	5726	+51
total			+102

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/securitysolution-utils`	46	54	+8

Unknown metric groups

API count

id	before	after	diff
`@kbn/securitysolution-utils`	51	60	+9

History

💛 Build #231731 was flaky 55305a3
💔 Build #231698 failed 5a1079b
💛 Build #230798 was flaky 83333b6
💛 Build #230420 was flaky 5438991
💔 Build #230287 failed f6c61bd
💔 Build #230261 failed b65f6a2

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

maximpn

@marshallmain thanks for addressing my comments 🙏

elasticmachine · 2024-09-06T13:42:35Z

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

…mpty lines in import list API (#192681) ## Summary The quickstart tooling introduced in #190634 uses axios under the hood to make requests to Kibana. When attaching file data to the axios request with `FormData`, axios adds an extra empty line after the end content boundary. The logic in `buffer_lines.ts` assumes that there are no more lines after the end content boundary line, so importing a list with the quickstart tooling would create a list with an extra empty item. This empty item fails validation when retrieved through other APIs. This PR prevents lines after the end content boundary from being turned into list items in the import list API.

…mpty lines in import list API (elastic#192681) ## Summary The quickstart tooling introduced in elastic#190634 uses axios under the hood to make requests to Kibana. When attaching file data to the axios request with `FormData`, axios adds an extra empty line after the end content boundary. The logic in `buffer_lines.ts` assumes that there are no more lines after the end content boundary line, so importing a list with the quickstart tooling would create a list with an extra empty item. This empty item fails validation when retrieved through other APIs. This PR prevents lines after the end content boundary from being turned into list items in the import list API. (cherry picked from commit 5f83ac0)

… for empty lines in import list API (#192681) (#194470) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution][Detection Engine] Avoid creating list items for empty lines in import list API (#192681)](#192681)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Marshall Main <55718608+marshallmain@users.noreply.github.com>

marshallmain added 3 commits August 15, 2024 10:07

First pass of quickstart scripts

0459ade

Add README

eac9744

Update README

214e2df

marshallmain marked this pull request as ready for review August 15, 2024 19:30

marshallmain requested review from a team as code owners August 15, 2024 19:30

marshallmain requested review from xcrzx and dhurley14 August 15, 2024 19:30

Merge branch 'main' into secsol-quickstart

36765ae

marshallmain added the release_note:skip Skip the PR/issue when compiling release notes label Aug 15, 2024

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

8ac381d

…fix'

jbudz approved these changes Aug 15, 2024

View reviewed changes

kibanamachine and others added 3 commits August 15, 2024 20:25

[CI] Auto-commit changed files from 'yarn openapi:generate'

e9f509f

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

6c19227

…-fix'

Fix typecheck by adding example client usages

59d4763

pheyos approved these changes Aug 20, 2024

View reviewed changes

marshallmain and others added 2 commits August 26, 2024 10:17

Add helper to create value list exception easily

21b6540

Merge branch 'main' into secsol-quickstart

d46d462

marshallmain commented Aug 26, 2024

View reviewed changes

banderror requested review from maximpn, jpdjere, nikitaindik and dplumlee August 26, 2024 14:59

marshallmain added 2 commits August 26, 2024 12:37

Update README with better module descriptions

e194525

Fix import

e9a7897

maximpn reviewed Aug 27, 2024

View reviewed changes

kibanamachine and others added 4 commits August 27, 2024 19:41

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

b65f6a2

…fix'

Another codegen fix

f6c61bd

Fix unused variable error

5438991

Merge branch 'main' into secsol-quickstart

83333b6

xcrzx requested review from maximpn and removed request for xcrzx August 30, 2024 14:13

maximpn reviewed Sep 2, 2024

View reviewed changes

marshallmain and others added 5 commits September 3, 2024 10:28

PR feedback: README update, remove clientConcurrency option, remove f…

b7f7867

…un variable name

Merge branch 'main' into secsol-quickstart

5a1079b

Remove outdated use of clientConcurrency

55305a3

Merge branch 'main' into secsol-quickstart

411bb83

[CI] Auto-commit changed files from 'yarn openapi:generate'

bd10ce0

maximpn approved these changes Sep 6, 2024

View reviewed changes

marshallmain merged commit 3cc7029 into elastic:main Sep 6, 2024
41 checks passed

marshallmain deleted the secsol-quickstart branch September 6, 2024 13:42

kibanamachine added v8.16.0 backport:skip This commit does not require backporting labels Sep 6, 2024

marshallmain added Team:Detection Rule Management Security Detection Rule Management Team Team:Detection Engine Security Solution Detection Engine Area and removed backport:skip This commit does not require backporting labels Sep 6, 2024

kibanamachine added the backport:skip This commit does not require backporting label Sep 6, 2024

marshallmain mentioned this pull request Sep 12, 2024

[Security Solution][Detection Engine] Avoid creating list items for empty lines in import list API #192681

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Quickstart script tooling for Detections and Response #190634

[Security Solution] Quickstart script tooling for Detections and Response #190634

marshallmain commented Aug 15, 2024 •

edited

Loading

jbudz left a comment

pheyos left a comment

marshallmain Aug 26, 2024

marshallmain Aug 26, 2024

maximpn Sep 2, 2024

banderror commented Aug 26, 2024

maximpn left a comment

maximpn Aug 27, 2024

maximpn Aug 27, 2024

marshallmain Aug 27, 2024

maximpn Sep 2, 2024

maximpn Sep 2, 2024

maximpn left a comment

maximpn Sep 2, 2024

marshallmain Sep 3, 2024

maximpn Sep 2, 2024

maximpn Sep 2, 2024

kibana-ci commented Sep 5, 2024

API count

maximpn left a comment

elasticmachine commented Sep 6, 2024

[Security Solution] Quickstart script tooling for Detections and Response #190634

[Security Solution] Quickstart script tooling for Detections and Response #190634

Conversation

marshallmain commented Aug 15, 2024 • edited Loading

Summary

jbudz left a comment

Choose a reason for hiding this comment

pheyos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banderror commented Aug 26, 2024

maximpn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maximpn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Sep 5, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

API count

History

maximpn left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 6, 2024

marshallmain commented Aug 15, 2024 •

edited

Loading