-
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 Assistant] Adds client hooks and internal routes for managing Knowledge Base Entries #184974
Conversation
export const ELASTIC_AI_ASSISTANT_KNOWLEDGE_BASE_ENTRIES_URL = `${ELASTIC_AI_ASSISTANT_INTERNAL_URL}/knowledge_base/entries`; | ||
export const ELASTIC_AI_ASSISTANT_KNOWLEDGE_BASE_ENTRIES_URL_FIND = `${ELASTIC_AI_ASSISTANT_KNOWLEDGE_BASE_ENTRIES_URL}/_find`; | ||
export const ELASTIC_AI_ASSISTANT_KNOWLEDGE_BASE_ENTRIES_URL_BULK_ACTION = `${ELASTIC_AI_ASSISTANT_KNOWLEDGE_BASE_ENTRIES_URL}/_bulk_action`; |
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.
Moving to internal
routes and actually registering them. Routes contain logic for license/authUser/featureFlagEnabled checks.
@@ -17,6 +17,7 @@ import { ArrayFromString } from '@kbn/zod-helpers'; | |||
* version: 1 | |||
*/ | |||
|
|||
import { SortOrder } from '../common_attributes.gen'; |
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.
SortOrder
was common across kb, conversations, and anonymization, so extracted to common attributes.
export type FindKnowledgeBaseEntriesRequestQuery = z.infer< | ||
typeof FindKnowledgeBaseEntriesRequestQuery | ||
>; | ||
export const FindKnowledgeBaseEntriesRequestQuery = z.object({ |
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 would like to support semantic search in this API as well (already plumbed it out in kbDataClient over in the sec-labs-kb PR). Will probably expose something like search_type:semantic
param to use it, and then re-use filter
param. If there is a more standard pattern for supporting multiple search types in the same API I can use that.
Not sure how pagination and stable references work with semantic search, so should verify that behavior first.
/** | ||
* Helper to perform checks for authenticated user, capability, and license. Perform all or one | ||
* of the checks by providing relevant optional params. Check order is license, authenticated user, | ||
* then capability. |
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.
Noticed our routes are starting to bloat with common checks like license, authUser or if a feature flag is enabled, so I made this helper to do any or all of those checks in a single block.
export const createKnowledgeBaseEntryRoute = (router: ElasticAssistantPluginRouter): void => { | ||
router.versioned | ||
.post({ | ||
access: 'public', | ||
access: 'internal', |
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.
This route was not previously registered, but it is now as of this PR, so marking as internal
until we're ready.
const docs = (await docsLoader.load()) as Array<Document<Metadata>>; | ||
const languageDocs = (await languageLoader.load()) as Array<Document<Metadata>>; | ||
const rawExampleQueries = await exampleQueriesLoader.load(); |
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.
Will be removing castings over in the KB content PR (as loaders become generalized): #184885
@elasticmachine merge upstream |
Flaky Test Runner Stats🟠 Some tests failed. - kibana-flaky-test-suite-runner#6327[❌] x-pack/test/security_solution_api_integration/test_suites/genai/nlp_cleanup_task/basic_license_essentials_tier/configs/serverless.config.ts: 20/25 tests passed. |
resource, | ||
signal, | ||
}: ReadKnowledgeBaseRequestParams & { http: HttpSetup; signal?: AbortSignal | undefined }): Promise< | ||
ReadKnowledgeBaseResponse | IHttpFetchError |
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.
Consider allowing errors to propagate as an alternative to returning IHttpFetchError
in this function, and the others in this file
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.
This was one of the existing hooks that got moved, so will live as-is for the time being. The new hooks propagate errors and return default data so are easier to work with. Will update these as the the cleanup work around the KB continues.
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.
Thanks @spong for the new KB hooks and routes! 🙏
LGTM 🚀
@elasticmachine merge upstream |
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.
Thanks for creating the knowledge base routes @spong !
{ | ||
method: 'GET', | ||
version: API_VERSIONS.internal.v1, | ||
query, |
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.
In this case we only retrieve 100 knowledge base maximum and no pagination, right?
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.
Yeah, this is leftover from using the conversations API as a template. I will be sure to plumb through pagination to the hook here 👍
* @param {Object} options - The options object. | ||
* @param {HttpSetup} options.http - HttpSetup | ||
* @param {Function} [options.onFetch] - transformation function for kb entries fetch result | ||
* @param {AbortSignal} [options.signal] - AbortSignal |
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.
* @param {Object} options - The options object. | |
* @param {HttpSetup} options.http - HttpSetup | |
* @param {Function} [options.onFetch] - transformation function for kb entries fetch result | |
* @param {AbortSignal} [options.signal] - AbortSignal | |
* @param {HttpSetup} options.http - HttpSetup | |
* @param {AbortSignal} [options.signal] - AbortSignal |
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @spong |
…o support `IndexEntries` (#186566) ## Summary This is a follow-up to #184974 that updates the KB Entries API's (and underlying schema) to support `IndexEntries` in addition to `DocumentEntries`. `IndexEntries` are entries in the Knowledge Base that are not backed by an embedded raw text source, but rather by an entire Index or Data Stream. The user can set the data source name, the specific field to query (must be ELSER embeddings in this initial implementation), and a description for when the assistant should search this data source for Knowledge Base content. This essentially enables the user to create custom retrieval tools backed by their own data. The changes in this PR, as with the other recent KB enhancements, are behind the following feature flag: ``` xpack.securitySolution.enableExperimental: - 'assistantKnowledgeBaseByDefault' ``` however as code change is required to test the new mappings. For this you can update the `knowledgeBaseDataStream` in `x-pack/plugins/elastic_assistant/server/ai_assistant_service/index.ts` to ```ts this.knowledgeBaseDataStream = this.createDataStream({ resource: 'knowledgeBase', kibanaVersion: options.kibanaVersion, fieldMap: knowledgeBaseFieldMapV2, // Update this to the V2 mapping }); ``` Change set includes: - [X] ES Knowledge Base data stream schema and OAS has been updated to support `IndexEntries`. - [X] OAS schema files have been moved to the `/entries` sub-directory - [ ] Backend KB services have been updated to support `IndexEntries` - [X] Storage methods updated - [ ] Retrieval methods updated (will round out these endpoint when working the UI next) --- With these API changes, I've also introduced a few sample `*.http` files for easier development/testing. These files are supported out of the box in JetBrains IDE's or in VSCode with the [httpyac](https://httpyac.github.io/) (and many other) extensions. Since the configuration for these files includes a `-` in the name, that's why you'll see a few @elastic/kibana-operations files updated. You can read more about `http` files [here](https://www.jetbrains.com/help/webstorm/http-client-in-product-code-editor.html) and for the spec see this repo [here](https://github.com/JetBrains/http-request-in-editor-spec/blob/master/spec.md). If we find these useful, we could add support to our [OpenAPI Generator](https://openapi-generator.tech/docs/generators/jetbrains-http-client) to create these automatically. They currently live co-located next to the OAS and generated schema files here: ``` x-pack/packages/kbn-elastic-assistant-common/impl/schemas/knowledge_base/entries/bulk_crud_knowledge_base_entries_route.http x-pack/packages/kbn-elastic-assistant-common/impl/schemas/knowledge_base/entries/crud_knowledge_base_entries_route.http ``` and the main config here: ``` x-pack/packages/kbn-elastic-assistant-common/env/http-client.env.json ``` The `x-pack/packages/kbn-elastic-assistant-common/.gitignore` has been updated to ignore `http-client.private.env.json` files locally, which is how you can override the config as you'd like. This is helpful to add variables like `basePath` as below: ``` { "dev": { "basePath": "/kbn" } } ``` To use them, just open the corresponding `*.http` for the API you want to test, and click `Send`, and the response will open in another tab. Here is what that looks like for creating one of the new `IndexEntry` KB documents that have been introduced in this PR: <p align="center"> <img width="500" src="https://github.com/user-attachments/assets/c9e70d1a-28d2-4eb3-9853-ab6d8e1c7acf" /> </p> ### 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) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials * Feature currently behind feature flag. Documentation to be added before flag is removed. Tracked in elastic/security-docs#5337 - [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 <42973632+kibanamachine@users.noreply.github.com>
Summary
This PR adds client hooks and basic REST API's for accessing and mutating Knowledge Base Entries. This is in support of @angorayc building out the new Knowledge Base settings interface.
Change set includes:
x-pack/packages/kbn-elastic-assistant/impl/knowledge_base
to be co-located next to the API methods where we put all our other hooks:x-pack/packages/kbn-elastic-assistant/impl/assistant/api/knowledge_base
kbn-elastic-assistant/impl/assistant/api/index.tsx
and intox-pack/packages/kbn-elastic-assistant/impl/assistant/api/knowledge_base/api.tsx
find_knowledge_base_entries_route.schema.yaml
OAS for the supporting/internal/elastic_assistant/knowledge_base/entries/_find
routeSortOrder
out of existing OAS's into the sharedschemas/common_attributes.schema.yaml
Client Hooks & Routes
Adds new
useKnowledgeBaseEntries()
hook and corresponding/knowledge_base/entries/_find
route for returning paginated KB Entries to populate the KB table in settings. E.g.Sample Response
Response is the full newly created
entry
. Same format for the entry as above in the_find
API, and theKnowledgeBaseEntries
cache is invalidated.Adds new
useCreateKnowledgeBaseEntry()
hook and corresponding/knowledge_base/entries
route for creating new KB EntriesAdds new
useDeleteKnowledgeBaseEntries()
hook and corresponding/knowledge_base/entries/_bulk_action
route for deleting existing KB Entries. I left a TODO to plumb throughdelete_by_query
so we can add a filter bar to the table. Need to confirm if we can do pagination with similarity search as well.See
KnowledgeBaseEntryBulkCrudActionResponse
for response formats.KnowledgeBaseEntries
cache is invalidated upon delete.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