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

[Security Assistant] Adds new Knowledge Base Management Settings UI #192665

Merged
merged 43 commits into from
Sep 25, 2024

Conversation

spong
Copy link
Member

@spong spong commented Sep 12, 2024

Summary

This PR updates the Knowledge Base Management Settings page to use the new entries API introduced in #186566. Many thanks to @angorayc for her work on the Assistant Management Settings overhaul, and initial implementation of this new KB Management UI over in #186847.

Feature Flag & Setup

The changes in this PR, as with the other recent V2 KB enhancements, are behind the following feature flag:

xpack.securitySolution.enableExperimental:
  - 'assistantKnowledgeBaseByDefault'

They also require a code change in the AIAssistantService to enable the new mapping (since setup happens on plugin start before FF registration), so be sure to update fieldMap to knowledgeBaseFieldMapV2 below before testing:

This is no longer the case as of cdec104. Just changing the above feature flag is now sufficient, just note that if upgrading and the KB was previously setup, you'll need to manually delete the data stream (DELETE /_data_stream/.kibana-elastic-ai-assistant-knowledge-base-default) or the management table will be littered with the old ESQL docs instead of being a single aggregate entry.

Once configured, the new Knowledge Base Management Settings will become available in Stack Management. The old settings UI is currently still available via the Settings Modal, but will soon be removed and replaced with links to the new interface via the Assistant Settings Context Menu (replacing the existing cog). Please see the designs (Security GenAI / Unified AI Assistant) for all changes.

Important

There are no migrations in place between the legacy and v2 KB mappings, so be sure to start with a clean ES data directory.

Testing

To aid with developing the UI, I took the opportunity to start fleshing out the KB Entries API integration tests. These live in x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries, and are currently configured to only run on @ess, as running tiny_elser in serverless and MKI environments can be tricky (more on that later).

To start the server and run the tests, from the x-pack/test/security_solution_api_integration/ directory run yarn genai_kb_entries:server:ess, and once started, yarn genai_kb_entries:runner:ess.

Changes in support of testing

In order to setup the API integration tests for use with the Knowledge Base, some functional changes needed to be made to the assistant/config:

  1. Since ELSER is a heavy model to run in CI, the ML folks have created pt_tiny_elser for use in testing. Unfortunately, the getELSER() helper off the ml client that we use to get the modelld for installing ELSER, ingest pipelines, etc, cannot be overridden (#193633), so we must have some other means of doing that. So to get things working in the test env, I've plumbed through an optional modelId override to the POST knowledge base route (/ internal/ elastic_assistant/ knowledge_base/{resource?}?modelId=pt_tiny_elser). This then overrides the aiAssistantService getELSER() function when fetching a kbDataClient using the request, which appears to be the only way to also trigger a reinitialization of the ingest pipeline (which required the modelId), since that usually only occurs on plugin start. If there is a cleaner way to perform this reinitialization, please let me know!

  2. Turns out getService('ml').importTrainedModel() can't be run in test env's with ssl:true, which is the default security config. You can read more about that issue in #193477, but the current workaround is to turn off ssl for this specific test configuration, so that's why ess.config.ts looks a little different. If there's a better way to manage this config, also please let me know!

Additional notes

We don't currently have a securityAssistant API client/service to use in integration tests, so I've just been creating one-off functions using supertest for now. I don't have the bandwidth to work this now, but perhaps @MadameSheema / @muskangulati-qasource could lend a hand here? I did need to test multi-user and multi-space scenarios, so I ported over the same auth helpers I saw used in other suites. Would be nice if these were bundled into the client as well ala how the o11y folks have done it here. Perhaps this is also on the list of things for @maximpn to generate from OAS's.... 🙃

RBAC

In plumbing the UI, I've tried to place // TODO: KB-RBAC tags in all the places I came across that will require an RBAC check/change. This includes some of the API integration tests, which I currently have skipped as they would fail without RBAC.

Other notable changes

  • There are now dedicated legacy and v2 helper functions when managing persistence/retrieval of knowledge base entries. This should help with tearing out the old KB later, and better readability now.
  • I've tried to remove dependency on the ElasticsearchStore as much as possible. The store's only use should now be within tools as a retriever here, and in post_evaluate here. If we adopt the new naturalLanguageToESQL tool in 8.16 (or update our existing ESQL tool to use the kbDataClient for retrieval), we should be able to get rid of this entirely.
  • Added a spaces_roles_users_data.http file for adding spaces, roles, users, and a sample slackbot index for use with sample IndexEntries here.

// TODO

In effort to make incremental progress and facilitate early knowledge share with @patrykkopycinski, I'm capping this PR where it's at, and so here are the remaining items to complete full integration of the new Knowledge Base Management Settings interface:

  • Support Update action
  • Move from EuiInMemoryTable
  • Finalize Setup UI
  • Cleanup Save loaders
  • Plumb through {{knowledge_history}} prompt template and include use's required entries

All this work is behind the aforementioned feature flag and required code change, and this changeset has also been manually upgrade tested to ensure there are no issues that would impact the regularly scheduled serverless releases. This is more of a note to reviewers when testing that full functionality is not present.

Checklist

@spong spong self-assigned this Sep 12, 2024
Comment on lines +22 to +29
// ssl: false as ML vocab API is broken with SSL enabled
.filter(
(a: string) =>
!(
a.startsWith('--elasticsearch.hosts=') ||
a.startsWith('--elasticsearch.ssl.certificateAuthorities=')
)
),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in the description, we must run with ssl:false for certain ml API's to function. See #193477 for details.

@andrew-goldstein
Copy link
Contributor

If the user is an admin (future RBAC setting enabled), they should see all users entries in KB settings, however their assistant sessions should only ever return their own and global entries. If the user is not an admin, they should only see their own and global entries in both KB settings and assistant sessions

Allowing (only) admin users to view the Private KB entries of other users feels inconsistent with the way conversations work:

  • Conversations are scoped to a user + space (this applies to all users)
    • Even users with superuser ES role don't have a CRUD UI to manage other users conversations
  • Private KB entries are (as expected) also scoped to a user + space, just like conversations
    • When users with the superuser role create private KB entries, those entries don't effect other users

What is the use case for an admin user changing a Private KB entry on behalf of another user, when those entries only effect the user who created them?

…age if FF disabled, fixed modelId override only if FF is enabled, and renamed assistant_all user
@spong
Copy link
Member Author

spong commented Sep 24, 2024

What is the use case for an admin user changing a Private KB entry on behalf of another user, when those entries only effect the user who created them?

I was mistaken in my original message, you are correct here @andrew-goldstein -- thank you for questioning this behavior! 🙏

I have confirmed with @jamesspi that RBAC should behave as following:

There will be a new sub-feature added (next to the anonymization sub-feature), that says something like Manage Global Knowledge Base, and when enabled, this feature allows the user to create global entries that everyone in the given space can see, but not edit unless they also have this feature. This feature grants no visibility into the private entries that any individual user has created. So any private entry a user makes can only be seen by that user.

As discussed in our pair session with @patrykkopycinski, I will re-work the _find API to behave accordingly. Till RBAC is put in place, the default behavior will be that all users will have global privileges. So the functional change here is that other users' private entries will not be displayed.

I will update and un-skip this API integration test to ensure this behavior is correct so we have this confidence going forward.

edit: this is done and test un-skipped and new test added in 4d151a5

[setEntry]
);
// TODO: KB-RBAC Disable global option if no RBAC
const sharingOptions = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Consider memoizing this non-primitive value, because it's passed as a prop to a component below

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do! I will be extracting the whole sharing field into its own component in the next UI PR as it's used in both entry types.

return onIndexClicked || onDocumentClicked ? (
<EuiPopover
button={
<EuiButton iconType="arrowDown" iconSide="right" onClick={onButtonClick}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider disabling the New button when the KB hasn't been setup yet to avoid the error toaster in the screenshot below:

error_creating_kb_entry

I'm also wondering if the Setup Knowledge Base CTA positioned at the end of the sentence in the screenshot above should be the same size text as the Learn more link, or perhaps be more prominent like it is in the assistant:

assistant_setup_button

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will disable the New button if not available 👍

And there's still some pending UI changes around the setup button. I chatted with @bojanasan and we decided to hide the table and show the same button as in the assistant as you recommend here.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 25, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
integrationAssistant 557 559 +2
securitySolution 5817 5829 +12
total +14

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 363 366 +3

Async chunks

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

id before after diff
securitySolution 20.4MB 20.5MB +19.6KB
Unknown metric groups

API count

id before after diff
@kbn/elastic-assistant-common 390 393 +3

ESLint disabled line counts

id before after diff
@kbn/elastic-assistant 21 20 -1
elasticAssistant 36 35 -1
total -2

References to deprecated APIs

id before after diff
securitySolution 456 457 +1

Total ESLint disabled count

id before after diff
@kbn/elastic-assistant 22 21 -1
elasticAssistant 36 35 -1
total -2

History

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

cc @spong

Copy link
Contributor

@andrew-goldstein andrew-goldstein left a 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 settings UI! 🙏
✅ Desk tested locally
LGTM 🚀

@spong spong merged commit 63730ea into elastic:main Sep 25, 2024
55 checks passed
@spong spong deleted the kb-settings-v2 branch September 25, 2024 20:38
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 192665

Questions ?

Please refer to the Backport tool documentation

@spong
Copy link
Member Author

spong commented Sep 25, 2024

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

spong added a commit to spong/kibana that referenced this pull request Sep 25, 2024
…lastic#192665)

## Summary

This PR updates the Knowledge Base Management Settings page to use the
new `entries` API introduced in
elastic#186566. Many thanks to @angorayc
for her work on the Assistant Management Settings overhaul, and initial
implementation of this new KB Management UI over in
elastic#186847.

<p align="center">
<img width="600"
src="https://github.com/user-attachments/assets/0a82587e-f33c-45f1-9165-1a676d6db5fa"
/>
</p>

### Feature Flag & Setup
The changes in this PR, as with the other [recent V2 KB
enhancements](elastic#186566), are behind
the following feature flag:
```
xpack.securitySolution.enableExperimental:
  - 'assistantKnowledgeBaseByDefault'
```

~They also require a code change in the `AIAssistantService` to enable
the new mapping (since setup happens on plugin start before FF
registration), so be sure to update `fieldMap` to
`knowledgeBaseFieldMapV2` below before testing:~

This is no longer the case as of
[cdec104](elastic@cdec104).
Just changing the above feature flag is now sufficient, just note that
if upgrading and the KB was previously setup, you'll need to manually
delete the data stream (`DELETE
/_data_stream/.kibana-elastic-ai-assistant-knowledge-base-default`) or
the management table will be littered with the old ESQL docs instead of
being a single aggregate entry.

Once configured, the new Knowledge Base Management Settings will become
available in Stack Management. The old settings UI is currently still
available via the Settings Modal, but will soon be removed and replaced
with links to the new interface via the Assistant Settings Context Menu
(replacing the existing `cog`). Please see the designs ([Security
GenAI](https://www.figma.com/design/BMvpY9EhcPIaoOS7LSrkL0/%5B8.15%2C-%5D-GenAI-Security-Settings?node-id=51-25207&node-type=canvas&t=t3vZSPhMxQhScJVt-0)
/ [Unified AI
Assistant](https://www.figma.com/design/xN20zMRNtMlirWB6n9n1xJ/Unified-AI-Assistant-Settings?node-id=0-1&node-type=canvas&t=3RDYE7h2DjLlFlcN-0))
for all changes.

> [!IMPORTANT]
> There are no migrations in place between the legacy and v2 KB
mappings, so be sure to start with a clean ES data directory.

### Testing

To aid with developing the UI, I took the opportunity to start fleshing
out the KB Entries API integration tests. These live in
[x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries](https://github.com/spong/kibana/tree/7ae6be136ad992b2163df13b55118556b01b6cb9/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries),
and are currently configured to only run on `@ess`, as running
`tiny_elser` in serverless and MKI environments can be tricky (more on
that later).

To start the server and run the tests, from the
`x-pack/test/security_solution_api_integration/` directory run `yarn
genai_kb_entries:server:ess`, and once started, `yarn
genai_kb_entries:runner:ess`.

##### Changes in support of testing

In order to setup the API integration tests for use with the Knowledge
Base, some functional changes needed to be made to the assistant/config:

1. Since ELSER is a heavy model to run in CI, the ML folks have created
`pt_tiny_elser` for use in testing. Unfortunately, the `getELSER()`
helper off the `ml` client that we use to get the `modelld` for
installing ELSER, ingest pipelines, etc, cannot be overridden
([elastic#193633](elastic#193633)), so we must
have some other means of doing that. So to get things working in the
test env, I've plumbed through an optional `modelId` override to the
POST knowledge base route (`/ internal/ elastic_assistant/
knowledge_base/{resource?}?modelId=pt_tiny_elser`). This then overrides
the aiAssistantService `getELSER()` function [when
fetching](https://github.com/elastic/kibana/blob/645b3b863be16d70b8a7130a84b248c19729c340/x-pack/plugins/elastic_assistant/server/ai_assistant_service/index.ts#L334-L354)
a `kbDataClient` using the request, which appears to be the only way to
also trigger a reinitialization of the ingest pipeline (which required
the `modelId`), since that usually only occurs on plugin start. If there
is a cleaner way to perform this reinitialization, please let me know!

2. Turns out
[`getService('ml').importTrainedModel()`](https://github.com/elastic/kibana/blob/f18224c6869ae52228da3764ca9a427106b872fb/x-pack/test/functional/services/ml/api.ts#L1575-L1587)
can't be run in test env's with `ssl:true`, which is the default
security config. You can read more about that issue in
[elastic#193477](elastic#193477), but the
current workaround is to turn off `ssl` for this specific test
configuration, so that's why
[`ess.config.ts`](https://github.com/spong/kibana/blob/cf73d4c7fcd69207a9625046456a94212da833c7/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/trial_license_complete_tier/configs/ess.config.ts#L22)
looks a little different. If there's a better way to manage this config,
also please let me know!

##### Additional notes

We don't currently have a `securityAssistant` API client/service to use
in integration tests, so I've just been creating one-off functions using
`supertest` for now. I don't have the bandwidth to work this now, but
perhaps @MadameSheema / @muskangulati-qasource could lend a hand here? I
did need to test multi-user and multi-space scenarios, so I ported over
the same [auth
helpers](https://github.com/elastic/kibana/tree/dc26f1012f35c2445028a87dcc8cb3f063e058b0/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/utils/auth)
I saw used in other suites. Would be nice if these were bundled into the
client as well ala how the o11y folks have done it
[here](https://github.com/elastic/kibana/blob/e9f23aa98e3abadd491be61b17e7daa3cc110cdb/x-pack/test/observability_ai_assistant_api_integration/tests/knowledge_base/knowledge_base.spec.ts#L27-L34).
Perhaps this is also on the list of things for @maximpn to generate from
OAS's.... 🙃

### RBAC
In plumbing the UI, I've tried to place `// TODO: KB-RBAC` tags in all
the places I came across that will require an RBAC check/change. This
includes some of the API integration tests, which I currently have
skipped as they would fail without RBAC.

### Other notable changes

* There are now dedicated `legacy` and `v2` helper functions when
managing persistence/retrieval of knowledge base entries. This should
help with tearing out the old KB later, and better readability now.
* I've tried to remove dependency on the `ElasticsearchStore` as much as
possible. The store's only use should now be within tools as a retriever
[here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/helpers.ts#L397-L405),
and in post_evaluate
[here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/evaluate/post_evaluate.ts#L170-L179).
If we adopt the new
[`naturalLanguageToESQL`](elastic#192042)
tool in `8.16` (or update our existing ESQL tool to use the
`kbDataClient` for retrieval), we should be able to get rid of this
entirely.
* Added a
[`spaces_roles_users_data.http`](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/utils/spaces_roles_users_data.http#L1)
file for adding spaces, roles, users, and a sample `slackbot` index for
use with [sample `IndexEntries`
here](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/schemas/knowledge_base/entries/crud_knowledge_base_entries_route.http#L18-L56).

### // TODO
In effort to make incremental progress and facilitate early knowledge
share with @patrykkopycinski, I'm capping this PR where it's at, and so
here are the remaining items to complete full integration of the new
Knowledge Base Management Settings interface:

- [ ] Support `Update` action
- [ ] Move from `EuiInMemoryTable`
- [ ] Finalize `Setup` UI
- [ ] Cleanup `Save` loaders
- [ ] Plumb through `{{knowledge_history}}` prompt template and include
use's `required` entries

All this work is behind the aforementioned feature flag and required
code change, and this changeset has also been manually upgrade tested to
ensure there are no issues that would impact the regularly scheduled
serverless releases. This is more of a note to reviewers when testing
that full functionality is not present.

### Checklist

- [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>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit 63730ea)
spong added a commit that referenced this pull request Sep 26, 2024
…s UI (#192665) (#194074)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Assistant] Adds new Knowledge Base Management Settings UI
(#192665)](#192665)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Garrett
Spong","email":"spong@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-09-25T20:38:18Z","message":"[Security
Assistant] Adds new Knowledge Base Management Settings UI
(#192665)\n\n## Summary\r\n\r\nThis PR updates the Knowledge Base
Management Settings page to use the\r\nnew `entries` API introduced
in\r\nhttps://github.com//pull/186566. Many thanks to
@angorayc\r\nfor her work on the Assistant Management Settings overhaul,
and initial\r\nimplementation of this new KB Management UI over
in\r\nhttps://github.com//pull/186847.\r\n\r\n<p
align=\"center\">\r\n<img
width=\"600\"\r\nsrc=\"https://github.com/user-attachments/assets/0a82587e-f33c-45f1-9165-1a676d6db5fa\"\r\n/>\r\n</p>
\r\n\r\n\r\n\r\n### Feature Flag & Setup\r\nThe changes in this PR, as
with the other [recent V2
KB\r\nenhancements](#186566), are
behind\r\nthe following feature
flag:\r\n```\r\nxpack.securitySolution.enableExperimental:\r\n -
'assistantKnowledgeBaseByDefault'\r\n```\r\n\r\n~They also require a
code change in the `AIAssistantService` to enable\r\nthe new mapping
(since setup happens on plugin start before FF\r\nregistration), so be
sure to update `fieldMap` to\r\n`knowledgeBaseFieldMapV2` below before
testing:~\r\n\r\nThis is no longer the case as
of\r\n[cdec104](https://github.com/elastic/kibana/pull/192665/commits/cdec10402f2e9b889598693f9f415c98ccd9855c).\r\nJust
changing the above feature flag is now sufficient, just note that\r\nif
upgrading and the KB was previously setup, you'll need to
manually\r\ndelete the data stream
(`DELETE\r\n/_data_stream/.kibana-elastic-ai-assistant-knowledge-base-default`)
or\r\nthe management table will be littered with the old ESQL docs
instead of\r\nbeing a single aggregate entry.\r\n\r\nOnce configured,
the new Knowledge Base Management Settings will become\r\navailable in
Stack Management. The old settings UI is currently still\r\navailable
via the Settings Modal, but will soon be removed and replaced\r\nwith
links to the new interface via the Assistant Settings Context
Menu\r\n(replacing the existing `cog`). Please see the designs
([Security\r\nGenAI](https://www.figma.com/design/BMvpY9EhcPIaoOS7LSrkL0/%5B8.15%2C-%5D-GenAI-Security-Settings?node-id=51-25207&node-type=canvas&t=t3vZSPhMxQhScJVt-0)\r\n/
[Unified
AI\r\nAssistant](https://www.figma.com/design/xN20zMRNtMlirWB6n9n1xJ/Unified-AI-Assistant-Settings?node-id=0-1&node-type=canvas&t=3RDYE7h2DjLlFlcN-0))\r\nfor
all changes.\r\n\r\n> [!IMPORTANT]\r\n> There are no migrations in place
between the legacy and v2 KB\r\nmappings, so be sure to start with a
clean ES data directory.\r\n\r\n### Testing\r\n\r\nTo aid with
developing the UI, I took the opportunity to start fleshing\r\nout the
KB Entries API integration tests. These live
in\r\n[x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries](https://github.com/spong/kibana/tree/7ae6be136ad992b2163df13b55118556b01b6cb9/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries),\r\nand
are currently configured to only run on `@ess`, as
running\r\n`tiny_elser` in serverless and MKI environments can be tricky
(more on\r\nthat later).\r\n\r\nTo start the server and run the tests,
from the\r\n`x-pack/test/security_solution_api_integration/` directory
run `yarn\r\ngenai_kb_entries:server:ess`, and once started,
`yarn\r\ngenai_kb_entries:runner:ess`.\r\n\r\n##### Changes in support
of testing\r\n\r\nIn order to setup the API integration tests for use
with the Knowledge\r\nBase, some functional changes needed to be made to
the assistant/config:\r\n\r\n1. Since ELSER is a heavy model to run in
CI, the ML folks have created\r\n`pt_tiny_elser` for use in testing.
Unfortunately, the `getELSER()`\r\nhelper off the `ml` client that we
use to get the `modelld` for\r\ninstalling ELSER, ingest pipelines, etc,
cannot be
overridden\r\n([#193633](#193633)),
so we must\r\nhave some other means of doing that. So to get things
working in the\r\ntest env, I've plumbed through an optional `modelId`
override to the\r\nPOST knowledge base route (`/ internal/
elastic_assistant/\r\nknowledge_base/{resource?}?modelId=pt_tiny_elser`).
This then overrides\r\nthe aiAssistantService `getELSER()` function
[when\r\nfetching](https://github.com/elastic/kibana/blob/645b3b863be16d70b8a7130a84b248c19729c340/x-pack/plugins/elastic_assistant/server/ai_assistant_service/index.ts#L334-L354)\r\na
`kbDataClient` using the request, which appears to be the only way
to\r\nalso trigger a reinitialization of the ingest pipeline (which
required\r\nthe `modelId`), since that usually only occurs on plugin
start. If there\r\nis a cleaner way to perform this reinitialization,
please let me know!\r\n\r\n2. Turns
out\r\n[`getService('ml').importTrainedModel()`](https://github.com/elastic/kibana/blob/f18224c6869ae52228da3764ca9a427106b872fb/x-pack/test/functional/services/ml/api.ts#L1575-L1587)\r\ncan't
be run in test env's with `ssl:true`, which is the default\r\nsecurity
config. You can read more about that issue
in\r\n[#193477](#193477), but
the\r\ncurrent workaround is to turn off `ssl` for this specific
test\r\nconfiguration, so that's
why\r\n[`ess.config.ts`](https://github.com/spong/kibana/blob/cf73d4c7fcd69207a9625046456a94212da833c7/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/trial_license_complete_tier/configs/ess.config.ts#L22)\r\nlooks
a little different. If there's a better way to manage this
config,\r\nalso please let me know!\r\n\r\n##### Additional
notes\r\n\r\nWe don't currently have a `securityAssistant` API
client/service to use\r\nin integration tests, so I've just been
creating one-off functions using\r\n`supertest` for now. I don't have
the bandwidth to work this now, but\r\nperhaps @MadameSheema /
@muskangulati-qasource could lend a hand here? I\r\ndid need to test
multi-user and multi-space scenarios, so I ported over\r\nthe same
[auth\r\nhelpers](https://github.com/elastic/kibana/tree/dc26f1012f35c2445028a87dcc8cb3f063e058b0/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/utils/auth)\r\nI
saw used in other suites. Would be nice if these were bundled into
the\r\nclient as well ala how the o11y folks have done
it\r\n[here](https://github.com/elastic/kibana/blob/e9f23aa98e3abadd491be61b17e7daa3cc110cdb/x-pack/test/observability_ai_assistant_api_integration/tests/knowledge_base/knowledge_base.spec.ts#L27-L34).\r\nPerhaps
this is also on the list of things for @maximpn to generate
from\r\nOAS's.... 🙃\r\n\r\n### RBAC\r\nIn plumbing the UI, I've tried to
place `// TODO: KB-RBAC` tags in all\r\nthe places I came across that
will require an RBAC check/change. This\r\nincludes some of the API
integration tests, which I currently have\r\nskipped as they would fail
without RBAC.\r\n\r\n### Other notable changes\r\n\r\n* There are now
dedicated `legacy` and `v2` helper functions when\r\nmanaging
persistence/retrieval of knowledge base entries. This should\r\nhelp
with tearing out the old KB later, and better readability now.\r\n* I've
tried to remove dependency on the `ElasticsearchStore` as much
as\r\npossible. The store's only use should now be within tools as a
retriever\r\n[here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/helpers.ts#L397-L405),\r\nand
in
post_evaluate\r\n[here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/evaluate/post_evaluate.ts#L170-L179).\r\nIf
we adopt the
new\r\n[`naturalLanguageToESQL`](https://github.com/elastic/kibana/pull/192042)\r\ntool
in `8.16` (or update our existing ESQL tool to use the\r\n`kbDataClient`
for retrieval), we should be able to get rid of this\r\nentirely.\r\n*
Added
a\r\n[`spaces_roles_users_data.http`](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/utils/spaces_roles_users_data.http#L1)\r\nfile
for adding spaces, roles, users, and a sample `slackbot` index
for\r\nuse with [sample
`IndexEntries`\r\nhere](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/schemas/knowledge_base/entries/crud_knowledge_base_entries_route.http#L18-L56).\r\n\r\n###
// TODO\r\nIn effort to make incremental progress and facilitate early
knowledge\r\nshare with @patrykkopycinski, I'm capping this PR where
it's at, and so\r\nhere are the remaining items to complete full
integration of the new\r\nKnowledge Base Management Settings
interface:\r\n\r\n- [ ] Support `Update` action\r\n- [ ] Move from
`EuiInMemoryTable` \r\n- [ ] Finalize `Setup` UI\r\n- [ ] Cleanup `Save`
loaders\r\n- [ ] Plumb through `{{knowledge_history}}` prompt template
and include\r\nuse's `required` entries\r\n\r\nAll this work is behind
the aforementioned feature flag and required\r\ncode change, and this
changeset has also been manually upgrade tested to\r\nensure there are
no issues that would impact the regularly scheduled\r\nserverless
releases. This is more of a note to reviewers when testing\r\nthat full
functionality is not present.\r\n\r\n\r\n\r\n\r\n### Checklist\r\n\r\n-
[X] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n* Feature
currently behind feature flag. Documentation to be added\r\nbefore flag
is removed. Tracked
in\r\nhttps://github.com/elastic/security-docs/issues/5337\r\n- [X]
[Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"63730ea0c9d9b036a05cb919b25b6d19c2ea8f03","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Feature:Security
Assistant","Team:Security Generative
AI","v8.16.0","backport:version"],"number":192665,"url":"https://github.com/elastic/kibana/pull/192665","mergeCommit":{"message":"[Security
Assistant] Adds new Knowledge Base Management Settings UI
(#192665)\n\n## Summary\r\n\r\nThis PR updates the Knowledge Base
Management Settings page to use the\r\nnew `entries` API introduced
in\r\nhttps://github.com//pull/186566. Many thanks to
@angorayc\r\nfor her work on the Assistant Management Settings overhaul,
and initial\r\nimplementation of this new KB Management UI over
in\r\nhttps://github.com//pull/186847.\r\n\r\n<p
align=\"center\">\r\n<img
width=\"600\"\r\nsrc=\"https://github.com/user-attachments/assets/0a82587e-f33c-45f1-9165-1a676d6db5fa\"\r\n/>\r\n</p>
\r\n\r\n\r\n\r\n### Feature Flag & Setup\r\nThe changes in this PR, as
with the other [recent V2
KB\r\nenhancements](#186566), are
behind\r\nthe following feature
flag:\r\n```\r\nxpack.securitySolution.enableExperimental:\r\n -
'assistantKnowledgeBaseByDefault'\r\n```\r\n\r\n~They also require a
code change in the `AIAssistantService` to enable\r\nthe new mapping
(since setup happens on plugin start before FF\r\nregistration), so be
sure to update `fieldMap` to\r\n`knowledgeBaseFieldMapV2` below before
testing:~\r\n\r\nThis is no longer the case as
of\r\n[cdec104](https://github.com/elastic/kibana/pull/192665/commits/cdec10402f2e9b889598693f9f415c98ccd9855c).\r\nJust
changing the above feature flag is now sufficient, just note that\r\nif
upgrading and the KB was previously setup, you'll need to
manually\r\ndelete the data stream
(`DELETE\r\n/_data_stream/.kibana-elastic-ai-assistant-knowledge-base-default`)
or\r\nthe management table will be littered with the old ESQL docs
instead of\r\nbeing a single aggregate entry.\r\n\r\nOnce configured,
the new Knowledge Base Management Settings will become\r\navailable in
Stack Management. The old settings UI is currently still\r\navailable
via the Settings Modal, but will soon be removed and replaced\r\nwith
links to the new interface via the Assistant Settings Context
Menu\r\n(replacing the existing `cog`). Please see the designs
([Security\r\nGenAI](https://www.figma.com/design/BMvpY9EhcPIaoOS7LSrkL0/%5B8.15%2C-%5D-GenAI-Security-Settings?node-id=51-25207&node-type=canvas&t=t3vZSPhMxQhScJVt-0)\r\n/
[Unified
AI\r\nAssistant](https://www.figma.com/design/xN20zMRNtMlirWB6n9n1xJ/Unified-AI-Assistant-Settings?node-id=0-1&node-type=canvas&t=3RDYE7h2DjLlFlcN-0))\r\nfor
all changes.\r\n\r\n> [!IMPORTANT]\r\n> There are no migrations in place
between the legacy and v2 KB\r\nmappings, so be sure to start with a
clean ES data directory.\r\n\r\n### Testing\r\n\r\nTo aid with
developing the UI, I took the opportunity to start fleshing\r\nout the
KB Entries API integration tests. These live
in\r\n[x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries](https://github.com/spong/kibana/tree/7ae6be136ad992b2163df13b55118556b01b6cb9/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries),\r\nand
are currently configured to only run on `@ess`, as
running\r\n`tiny_elser` in serverless and MKI environments can be tricky
(more on\r\nthat later).\r\n\r\nTo start the server and run the tests,
from the\r\n`x-pack/test/security_solution_api_integration/` directory
run `yarn\r\ngenai_kb_entries:server:ess`, and once started,
`yarn\r\ngenai_kb_entries:runner:ess`.\r\n\r\n##### Changes in support
of testing\r\n\r\nIn order to setup the API integration tests for use
with the Knowledge\r\nBase, some functional changes needed to be made to
the assistant/config:\r\n\r\n1. Since ELSER is a heavy model to run in
CI, the ML folks have created\r\n`pt_tiny_elser` for use in testing.
Unfortunately, the `getELSER()`\r\nhelper off the `ml` client that we
use to get the `modelld` for\r\ninstalling ELSER, ingest pipelines, etc,
cannot be
overridden\r\n([#193633](#193633)),
so we must\r\nhave some other means of doing that. So to get things
working in the\r\ntest env, I've plumbed through an optional `modelId`
override to the\r\nPOST knowledge base route (`/ internal/
elastic_assistant/\r\nknowledge_base/{resource?}?modelId=pt_tiny_elser`).
This then overrides\r\nthe aiAssistantService `getELSER()` function
[when\r\nfetching](https://github.com/elastic/kibana/blob/645b3b863be16d70b8a7130a84b248c19729c340/x-pack/plugins/elastic_assistant/server/ai_assistant_service/index.ts#L334-L354)\r\na
`kbDataClient` using the request, which appears to be the only way
to\r\nalso trigger a reinitialization of the ingest pipeline (which
required\r\nthe `modelId`), since that usually only occurs on plugin
start. If there\r\nis a cleaner way to perform this reinitialization,
please let me know!\r\n\r\n2. Turns
out\r\n[`getService('ml').importTrainedModel()`](https://github.com/elastic/kibana/blob/f18224c6869ae52228da3764ca9a427106b872fb/x-pack/test/functional/services/ml/api.ts#L1575-L1587)\r\ncan't
be run in test env's with `ssl:true`, which is the default\r\nsecurity
config. You can read more about that issue
in\r\n[#193477](#193477), but
the\r\ncurrent workaround is to turn off `ssl` for this specific
test\r\nconfiguration, so that's
why\r\n[`ess.config.ts`](https://github.com/spong/kibana/blob/cf73d4c7fcd69207a9625046456a94212da833c7/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/trial_license_complete_tier/configs/ess.config.ts#L22)\r\nlooks
a little different. If there's a better way to manage this
config,\r\nalso please let me know!\r\n\r\n##### Additional
notes\r\n\r\nWe don't currently have a `securityAssistant` API
client/service to use\r\nin integration tests, so I've just been
creating one-off functions using\r\n`supertest` for now. I don't have
the bandwidth to work this now, but\r\nperhaps @MadameSheema /
@muskangulati-qasource could lend a hand here? I\r\ndid need to test
multi-user and multi-space scenarios, so I ported over\r\nthe same
[auth\r\nhelpers](https://github.com/elastic/kibana/tree/dc26f1012f35c2445028a87dcc8cb3f063e058b0/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/utils/auth)\r\nI
saw used in other suites. Would be nice if these were bundled into
the\r\nclient as well ala how the o11y folks have done
it\r\n[here](https://github.com/elastic/kibana/blob/e9f23aa98e3abadd491be61b17e7daa3cc110cdb/x-pack/test/observability_ai_assistant_api_integration/tests/knowledge_base/knowledge_base.spec.ts#L27-L34).\r\nPerhaps
this is also on the list of things for @maximpn to generate
from\r\nOAS's.... 🙃\r\n\r\n### RBAC\r\nIn plumbing the UI, I've tried to
place `// TODO: KB-RBAC` tags in all\r\nthe places I came across that
will require an RBAC check/change. This\r\nincludes some of the API
integration tests, which I currently have\r\nskipped as they would fail
without RBAC.\r\n\r\n### Other notable changes\r\n\r\n* There are now
dedicated `legacy` and `v2` helper functions when\r\nmanaging
persistence/retrieval of knowledge base entries. This should\r\nhelp
with tearing out the old KB later, and better readability now.\r\n* I've
tried to remove dependency on the `ElasticsearchStore` as much
as\r\npossible. The store's only use should now be within tools as a
retriever\r\n[here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/helpers.ts#L397-L405),\r\nand
in
post_evaluate\r\n[here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/evaluate/post_evaluate.ts#L170-L179).\r\nIf
we adopt the
new\r\n[`naturalLanguageToESQL`](https://github.com/elastic/kibana/pull/192042)\r\ntool
in `8.16` (or update our existing ESQL tool to use the\r\n`kbDataClient`
for retrieval), we should be able to get rid of this\r\nentirely.\r\n*
Added
a\r\n[`spaces_roles_users_data.http`](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/utils/spaces_roles_users_data.http#L1)\r\nfile
for adding spaces, roles, users, and a sample `slackbot` index
for\r\nuse with [sample
`IndexEntries`\r\nhere](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/schemas/knowledge_base/entries/crud_knowledge_base_entries_route.http#L18-L56).\r\n\r\n###
// TODO\r\nIn effort to make incremental progress and facilitate early
knowledge\r\nshare with @patrykkopycinski, I'm capping this PR where
it's at, and so\r\nhere are the remaining items to complete full
integration of the new\r\nKnowledge Base Management Settings
interface:\r\n\r\n- [ ] Support `Update` action\r\n- [ ] Move from
`EuiInMemoryTable` \r\n- [ ] Finalize `Setup` UI\r\n- [ ] Cleanup `Save`
loaders\r\n- [ ] Plumb through `{{knowledge_history}}` prompt template
and include\r\nuse's `required` entries\r\n\r\nAll this work is behind
the aforementioned feature flag and required\r\ncode change, and this
changeset has also been manually upgrade tested to\r\nensure there are
no issues that would impact the regularly scheduled\r\nserverless
releases. This is more of a note to reviewers when testing\r\nthat full
functionality is not present.\r\n\r\n\r\n\r\n\r\n### Checklist\r\n\r\n-
[X] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n* Feature
currently behind feature flag. Documentation to be added\r\nbefore flag
is removed. Tracked
in\r\nhttps://github.com/elastic/security-docs/issues/5337\r\n- [X]
[Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"63730ea0c9d9b036a05cb919b25b6d19c2ea8f03"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192665","number":192665,"mergeCommit":{"message":"[Security
Assistant] Adds new Knowledge Base Management Settings UI
(#192665)\n\n## Summary\r\n\r\nThis PR updates the Knowledge Base
Management Settings page to use the\r\nnew `entries` API introduced
in\r\nhttps://github.com//pull/186566. Many thanks to
@angorayc\r\nfor her work on the Assistant Management Settings overhaul,
and initial\r\nimplementation of this new KB Management UI over
in\r\nhttps://github.com//pull/186847.\r\n\r\n<p
align=\"center\">\r\n<img
width=\"600\"\r\nsrc=\"https://github.com/user-attachments/assets/0a82587e-f33c-45f1-9165-1a676d6db5fa\"\r\n/>\r\n</p>
\r\n\r\n\r\n\r\n### Feature Flag & Setup\r\nThe changes in this PR, as
with the other [recent V2
KB\r\nenhancements](#186566), are
behind\r\nthe following feature
flag:\r\n```\r\nxpack.securitySolution.enableExperimental:\r\n -
'assistantKnowledgeBaseByDefault'\r\n```\r\n\r\n~They also require a
code change in the `AIAssistantService` to enable\r\nthe new mapping
(since setup happens on plugin start before FF\r\nregistration), so be
sure to update `fieldMap` to\r\n`knowledgeBaseFieldMapV2` below before
testing:~\r\n\r\nThis is no longer the case as
of\r\n[cdec104](https://github.com/elastic/kibana/pull/192665/commits/cdec10402f2e9b889598693f9f415c98ccd9855c).\r\nJust
changing the above feature flag is now sufficient, just note that\r\nif
upgrading and the KB was previously setup, you'll need to
manually\r\ndelete the data stream
(`DELETE\r\n/_data_stream/.kibana-elastic-ai-assistant-knowledge-base-default`)
or\r\nthe management table will be littered with the old ESQL docs
instead of\r\nbeing a single aggregate entry.\r\n\r\nOnce configured,
the new Knowledge Base Management Settings will become\r\navailable in
Stack Management. The old settings UI is currently still\r\navailable
via the Settings Modal, but will soon be removed and replaced\r\nwith
links to the new interface via the Assistant Settings Context
Menu\r\n(replacing the existing `cog`). Please see the designs
([Security\r\nGenAI](https://www.figma.com/design/BMvpY9EhcPIaoOS7LSrkL0/%5B8.15%2C-%5D-GenAI-Security-Settings?node-id=51-25207&node-type=canvas&t=t3vZSPhMxQhScJVt-0)\r\n/
[Unified
AI\r\nAssistant](https://www.figma.com/design/xN20zMRNtMlirWB6n9n1xJ/Unified-AI-Assistant-Settings?node-id=0-1&node-type=canvas&t=3RDYE7h2DjLlFlcN-0))\r\nfor
all changes.\r\n\r\n> [!IMPORTANT]\r\n> There are no migrations in place
between the legacy and v2 KB\r\nmappings, so be sure to start with a
clean ES data directory.\r\n\r\n### Testing\r\n\r\nTo aid with
developing the UI, I took the opportunity to start fleshing\r\nout the
KB Entries API integration tests. These live
in\r\n[x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries](https://github.com/spong/kibana/tree/7ae6be136ad992b2163df13b55118556b01b6cb9/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries),\r\nand
are currently configured to only run on `@ess`, as
running\r\n`tiny_elser` in serverless and MKI environments can be tricky
(more on\r\nthat later).\r\n\r\nTo start the server and run the tests,
from the\r\n`x-pack/test/security_solution_api_integration/` directory
run `yarn\r\ngenai_kb_entries:server:ess`, and once started,
`yarn\r\ngenai_kb_entries:runner:ess`.\r\n\r\n##### Changes in support
of testing\r\n\r\nIn order to setup the API integration tests for use
with the Knowledge\r\nBase, some functional changes needed to be made to
the assistant/config:\r\n\r\n1. Since ELSER is a heavy model to run in
CI, the ML folks have created\r\n`pt_tiny_elser` for use in testing.
Unfortunately, the `getELSER()`\r\nhelper off the `ml` client that we
use to get the `modelld` for\r\ninstalling ELSER, ingest pipelines, etc,
cannot be
overridden\r\n([#193633](#193633)),
so we must\r\nhave some other means of doing that. So to get things
working in the\r\ntest env, I've plumbed through an optional `modelId`
override to the\r\nPOST knowledge base route (`/ internal/
elastic_assistant/\r\nknowledge_base/{resource?}?modelId=pt_tiny_elser`).
This then overrides\r\nthe aiAssistantService `getELSER()` function
[when\r\nfetching](https://github.com/elastic/kibana/blob/645b3b863be16d70b8a7130a84b248c19729c340/x-pack/plugins/elastic_assistant/server/ai_assistant_service/index.ts#L334-L354)\r\na
`kbDataClient` using the request, which appears to be the only way
to\r\nalso trigger a reinitialization of the ingest pipeline (which
required\r\nthe `modelId`), since that usually only occurs on plugin
start. If there\r\nis a cleaner way to perform this reinitialization,
please let me know!\r\n\r\n2. Turns
out\r\n[`getService('ml').importTrainedModel()`](https://github.com/elastic/kibana/blob/f18224c6869ae52228da3764ca9a427106b872fb/x-pack/test/functional/services/ml/api.ts#L1575-L1587)\r\ncan't
be run in test env's with `ssl:true`, which is the default\r\nsecurity
config. You can read more about that issue
in\r\n[#193477](#193477), but
the\r\ncurrent workaround is to turn off `ssl` for this specific
test\r\nconfiguration, so that's
why\r\n[`ess.config.ts`](https://github.com/spong/kibana/blob/cf73d4c7fcd69207a9625046456a94212da833c7/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/trial_license_complete_tier/configs/ess.config.ts#L22)\r\nlooks
a little different. If there's a better way to manage this
config,\r\nalso please let me know!\r\n\r\n##### Additional
notes\r\n\r\nWe don't currently have a `securityAssistant` API
client/service to use\r\nin integration tests, so I've just been
creating one-off functions using\r\n`supertest` for now. I don't have
the bandwidth to work this now, but\r\nperhaps @MadameSheema /
@muskangulati-qasource could lend a hand here? I\r\ndid need to test
multi-user and multi-space scenarios, so I ported over\r\nthe same
[auth\r\nhelpers](https://github.com/elastic/kibana/tree/dc26f1012f35c2445028a87dcc8cb3f063e058b0/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/utils/auth)\r\nI
saw used in other suites. Would be nice if these were bundled into
the\r\nclient as well ala how the o11y folks have done
it\r\n[here](https://github.com/elastic/kibana/blob/e9f23aa98e3abadd491be61b17e7daa3cc110cdb/x-pack/test/observability_ai_assistant_api_integration/tests/knowledge_base/knowledge_base.spec.ts#L27-L34).\r\nPerhaps
this is also on the list of things for @maximpn to generate
from\r\nOAS's.... 🙃\r\n\r\n### RBAC\r\nIn plumbing the UI, I've tried to
place `// TODO: KB-RBAC` tags in all\r\nthe places I came across that
will require an RBAC check/change. This\r\nincludes some of the API
integration tests, which I currently have\r\nskipped as they would fail
without RBAC.\r\n\r\n### Other notable changes\r\n\r\n* There are now
dedicated `legacy` and `v2` helper functions when\r\nmanaging
persistence/retrieval of knowledge base entries. This should\r\nhelp
with tearing out the old KB later, and better readability now.\r\n* I've
tried to remove dependency on the `ElasticsearchStore` as much
as\r\npossible. The store's only use should now be within tools as a
retriever\r\n[here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/helpers.ts#L397-L405),\r\nand
in
post_evaluate\r\n[here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/evaluate/post_evaluate.ts#L170-L179).\r\nIf
we adopt the
new\r\n[`naturalLanguageToESQL`](https://github.com/elastic/kibana/pull/192042)\r\ntool
in `8.16` (or update our existing ESQL tool to use the\r\n`kbDataClient`
for retrieval), we should be able to get rid of this\r\nentirely.\r\n*
Added
a\r\n[`spaces_roles_users_data.http`](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/utils/spaces_roles_users_data.http#L1)\r\nfile
for adding spaces, roles, users, and a sample `slackbot` index
for\r\nuse with [sample
`IndexEntries`\r\nhere](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/schemas/knowledge_base/entries/crud_knowledge_base_entries_route.http#L18-L56).\r\n\r\n###
// TODO\r\nIn effort to make incremental progress and facilitate early
knowledge\r\nshare with @patrykkopycinski, I'm capping this PR where
it's at, and so\r\nhere are the remaining items to complete full
integration of the new\r\nKnowledge Base Management Settings
interface:\r\n\r\n- [ ] Support `Update` action\r\n- [ ] Move from
`EuiInMemoryTable` \r\n- [ ] Finalize `Setup` UI\r\n- [ ] Cleanup `Save`
loaders\r\n- [ ] Plumb through `{{knowledge_history}}` prompt template
and include\r\nuse's `required` entries\r\n\r\nAll this work is behind
the aforementioned feature flag and required\r\ncode change, and this
changeset has also been manually upgrade tested to\r\nensure there are
no issues that would impact the regularly scheduled\r\nserverless
releases. This is more of a note to reviewers when testing\r\nthat full
functionality is not present.\r\n\r\n\r\n\r\n\r\n### Checklist\r\n\r\n-
[X] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n* Feature
currently behind feature flag. Documentation to be added\r\nbefore flag
is removed. Tracked
in\r\nhttps://github.com/elastic/security-docs/issues/5337\r\n- [X]
[Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"63730ea0c9d9b036a05cb919b25b6d19c2ea8f03"}},{"branch":"8.x","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
patrykkopycinski added a commit that referenced this pull request Oct 9, 2024
…194354)

## Summary

This PR is a follow up to #192665 and addresses a bunch of feedback and
fixes including:

- [X] Adds support for updating/editing entries
- [X] Fixes initial loading experience of the KB Settings Setup/Table
- [X] Fixes two bugs where `semantic_text` and `text` must be declared
for `IndexEntries` to work
- [X] Add new Settings Context Menu items for KB and Alerts
 - [X] Add support for `required` entries in initial prompt
* See [this
trace](https://smith.langchain.com/public/84a17a31-8ce8-4bd9-911e-38a854484dd8/r)
for included knowledge. Note that the KnowledgeBaseRetrievalTool was not
selected.
* Note: All prompts were updated to include the `{knowledge_history}`
placeholder, and _not behind the feature flag_, as this will just be the
empty case until the feature flag is enabled.

TODO (in this or follow-up PR):
 - [ ] Add suggestions to `index` and `fields` inputs
 - [ ] Adds URL deeplinking to securityAssistantManagement
- [ ] Fix bug where updating entry does not re-create embeddings (see
[comment](#194354 (comment)))
 - [ ] Fix loading indicators when adding/editing entries
 - [ ] API integration tests for update API (@e40pud)


### 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
* Docs being tracked in
elastic/security-docs#5337 for when feature
flag is enabled
- [ ] [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>
Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 9, 2024
…lastic#194354)

## Summary

This PR is a follow up to elastic#192665 and addresses a bunch of feedback and
fixes including:

- [X] Adds support for updating/editing entries
- [X] Fixes initial loading experience of the KB Settings Setup/Table
- [X] Fixes two bugs where `semantic_text` and `text` must be declared
for `IndexEntries` to work
- [X] Add new Settings Context Menu items for KB and Alerts
 - [X] Add support for `required` entries in initial prompt
* See [this
trace](https://smith.langchain.com/public/84a17a31-8ce8-4bd9-911e-38a854484dd8/r)
for included knowledge. Note that the KnowledgeBaseRetrievalTool was not
selected.
* Note: All prompts were updated to include the `{knowledge_history}`
placeholder, and _not behind the feature flag_, as this will just be the
empty case until the feature flag is enabled.

TODO (in this or follow-up PR):
 - [ ] Add suggestions to `index` and `fields` inputs
 - [ ] Adds URL deeplinking to securityAssistantManagement
- [ ] Fix bug where updating entry does not re-create embeddings (see
[comment](elastic#194354 (comment)))
 - [ ] Fix loading indicators when adding/editing entries
 - [ ] API integration tests for update API (@e40pud)

### 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
* Docs being tracked in
elastic/security-docs#5337 for when feature
flag is enabled
- [ ] [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>
Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com>
(cherry picked from commit 7df3672)
kibanamachine added a commit that referenced this pull request Oct 9, 2024
…xes (#194354) (#195644)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Assistant] V2 Knowledge Base Settings feedback and fixes
(#194354)](#194354)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Garrett
Spong","email":"spong@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-10-09T16:17:47Z","message":"[Security
Assistant] V2 Knowledge Base Settings feedback and fixes (#194354)\n\n##
Summary\r\n\r\nThis PR is a follow up to #192665 and addresses a bunch
of feedback and\r\nfixes including:\r\n\r\n- [X] Adds support for
updating/editing entries\r\n- [X] Fixes initial loading experience of
the KB Settings Setup/Table\r\n- [X] Fixes two bugs where
`semantic_text` and `text` must be declared\r\nfor `IndexEntries` to
work\r\n- [X] Add new Settings Context Menu items for KB and Alerts\r\n
- [X] Add support for `required` entries in initial prompt\r\n* See
[this\r\ntrace](https://smith.langchain.com/public/84a17a31-8ce8-4bd9-911e-38a854484dd8/r)\r\nfor
included knowledge. Note that the KnowledgeBaseRetrievalTool was
not\r\nselected.\r\n* Note: All prompts were updated to include the
`{knowledge_history}`\r\nplaceholder, and _not behind the feature flag_,
as this will just be the\r\nempty case until the feature flag is
enabled.\r\n\r\nTODO (in this or follow-up PR):\r\n - [ ] Add
suggestions to `index` and `fields` inputs\r\n - [ ] Adds URL
deeplinking to securityAssistantManagement\r\n- [ ] Fix bug where
updating entry does not re-create embeddings
(see\r\n[comment](https://github.com/elastic/kibana/pull/194354#discussion_r1786475496))\r\n
- [ ] Fix loading indicators when adding/editing entries\r\n - [ ] API
integration tests for update API (@e40pud)\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [X] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n* Docs being
tracked in\r\nhttps://github.com/elastic/security-docs/issues/5337 for
when feature\r\nflag is enabled\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Patryk Kopycinski
<contact@patrykkopycinski.com>","sha":"7df36721923159f45bc4fdbd26f76b20ad84249a","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Feature:Security
Assistant","Team:Security Generative
AI","v8.16.0","backport:version"],"title":"[Security Assistant] V2
Knowledge Base Settings feedback and
fixes","number":194354,"url":"https://github.com/elastic/kibana/pull/194354","mergeCommit":{"message":"[Security
Assistant] V2 Knowledge Base Settings feedback and fixes (#194354)\n\n##
Summary\r\n\r\nThis PR is a follow up to #192665 and addresses a bunch
of feedback and\r\nfixes including:\r\n\r\n- [X] Adds support for
updating/editing entries\r\n- [X] Fixes initial loading experience of
the KB Settings Setup/Table\r\n- [X] Fixes two bugs where
`semantic_text` and `text` must be declared\r\nfor `IndexEntries` to
work\r\n- [X] Add new Settings Context Menu items for KB and Alerts\r\n
- [X] Add support for `required` entries in initial prompt\r\n* See
[this\r\ntrace](https://smith.langchain.com/public/84a17a31-8ce8-4bd9-911e-38a854484dd8/r)\r\nfor
included knowledge. Note that the KnowledgeBaseRetrievalTool was
not\r\nselected.\r\n* Note: All prompts were updated to include the
`{knowledge_history}`\r\nplaceholder, and _not behind the feature flag_,
as this will just be the\r\nempty case until the feature flag is
enabled.\r\n\r\nTODO (in this or follow-up PR):\r\n - [ ] Add
suggestions to `index` and `fields` inputs\r\n - [ ] Adds URL
deeplinking to securityAssistantManagement\r\n- [ ] Fix bug where
updating entry does not re-create embeddings
(see\r\n[comment](https://github.com/elastic/kibana/pull/194354#discussion_r1786475496))\r\n
- [ ] Fix loading indicators when adding/editing entries\r\n - [ ] API
integration tests for update API (@e40pud)\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [X] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n* Docs being
tracked in\r\nhttps://github.com/elastic/security-docs/issues/5337 for
when feature\r\nflag is enabled\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Patryk Kopycinski
<contact@patrykkopycinski.com>","sha":"7df36721923159f45bc4fdbd26f76b20ad84249a"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194354","number":194354,"mergeCommit":{"message":"[Security
Assistant] V2 Knowledge Base Settings feedback and fixes (#194354)\n\n##
Summary\r\n\r\nThis PR is a follow up to #192665 and addresses a bunch
of feedback and\r\nfixes including:\r\n\r\n- [X] Adds support for
updating/editing entries\r\n- [X] Fixes initial loading experience of
the KB Settings Setup/Table\r\n- [X] Fixes two bugs where
`semantic_text` and `text` must be declared\r\nfor `IndexEntries` to
work\r\n- [X] Add new Settings Context Menu items for KB and Alerts\r\n
- [X] Add support for `required` entries in initial prompt\r\n* See
[this\r\ntrace](https://smith.langchain.com/public/84a17a31-8ce8-4bd9-911e-38a854484dd8/r)\r\nfor
included knowledge. Note that the KnowledgeBaseRetrievalTool was
not\r\nselected.\r\n* Note: All prompts were updated to include the
`{knowledge_history}`\r\nplaceholder, and _not behind the feature flag_,
as this will just be the\r\nempty case until the feature flag is
enabled.\r\n\r\nTODO (in this or follow-up PR):\r\n - [ ] Add
suggestions to `index` and `fields` inputs\r\n - [ ] Adds URL
deeplinking to securityAssistantManagement\r\n- [ ] Fix bug where
updating entry does not re-create embeddings
(see\r\n[comment](https://github.com/elastic/kibana/pull/194354#discussion_r1786475496))\r\n
- [ ] Fix loading indicators when adding/editing entries\r\n - [ ] API
integration tests for update API (@e40pud)\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [X] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n* Docs being
tracked in\r\nhttps://github.com/elastic/security-docs/issues/5337 for
when feature\r\nflag is enabled\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Patryk Kopycinski
<contact@patrykkopycinski.com>","sha":"7df36721923159f45bc4fdbd26f76b20ad84249a"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels Feature:Security Assistant Security Assistant release_note:skip Skip the PR/issue when compiling release notes Team:Security Generative AI Security Generative AI v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants