Observability AI Assistant Tests Deployment Agnostic

[arturoliduena]

Closes #192718

Summary

This PR add a deployment-agnostic testing environment for Observability AI Assistant tests by unifying the duplicated tests for stateful and serverless environments. It create the ObservabilityAIAssistantApiClient to work seamlessly in both environments, enabling a single test to run across stateful, CI, and MKI.

Initial efforts focus on deduplicating the conversations.spec.ts and connectors.spec.ts files, as these already run in all environments.

Move / dedup the tests that exist in stateful and serverless. They run in serverless CI but not MKI and add the skipMki tag.
chat.spec.ts
complete.spec.ts
elasticsearch.spec.ts
public_complete.spec.ts
alerts.spec.ts
knowledge_base_setup.spec.ts
knowledge_base_status.spec.ts
knowledge_base.spec.ts
summarize.ts
knowledge_base_user_instructions.spec.ts

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
• Flaky Test Runner was used on any tests changed
• The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

[elasticmachine]

Pinging @elastic/obs-ai-assistant (Team:Obs AI Assistant)

[github-actions]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

• /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
• run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

[sorenlouv]

nit: I suggest not using variables here. It's harder to read and search for. Also, I'm not sure the typescript intellisense for parameters and typed responses works when using variables

Suggested change

	endpoint: `GET ${CONNECTOR_API_URL}`,
	endpoint: `GET /internal/observability_ai_assistant/connectors`,

[sorenlouv]

If you really want to make it DRY I suggest adding a helper and re-use that instead:

const getConnectors = () =>
  observabilityAIAssistantAPIClient.editor({
    endpoint: `GET /internal/observability_ai_assistant/connectors`,
  });

[arturoliduena]

Agree. Now that I’m moving many of these tests, I won’t use a variable and will ensure they follow a consistent pattern since we currently have a mix.

[sorenlouv]

Should we have a shared helper for creating the proxy action connector? It's already defined at least 3 times:

kibana/x-pack/test/observability_ai_assistant_api_integration/common/action_connectors.ts

Line 31 in c4cf9fe

export async function createProxyActionConnector({

kibana/x-pack/test_serverless/api_integration/test_suites/observability/ai_assistant/common/action_connectors.ts

Line 40 in c4cf9fe

export async function createProxyActionConnector({

kibana/x-pack/test/observability_ai_assistant_functional/common/connectors.ts

Line 10 in c4cf9fe

export async function createConnector(proxy: LlmProxy, supertest: SuperTestAgent) {

[arturoliduena]

I will keep only one, I will get rid of the duplicate functions.

[sorenlouv]

Feels good getting rid of the serverless-specific client. Is the plan to get rid of x-pack/test_serverless/api_integration/test_suites/observability/ai_assistant/common/observability_ai_assistant_api_client.ts entirely?

[neptunian]

We can remove it entirely if we get all the tests running in deployment agnostic, unless there is some case where we want a serverless only test because of some serverless only feature. Probably wouldn't hurt to leave it.

[neptunian]

This whole file should be safe to move over to deployment agnostic and not exist in stateful tests. I'm assuming you decided not to move it was because of the security roles and access privileges test suite which doesn't exist in serverless, but it soon will: #205210

[arturoliduena]

Exactly. I only moved the common tests between stateful and serverless. I’ll move the security roles and access privaleges to deployment-agnostic and remove it from stateful and serverless.

[neptunian]

Same as https://github.com/elastic/kibana/pull/205194/files#r1901174829

[sorenlouv]

Doesn't look related to your change but Github complains about errors in this file

[sorenlouv]

It's valid on main which is odd

[arturoliduena]

Thanks for pointing that out, @sorenlouv. I checked, and the file is now valid in this branch as well. It might have been a temporary issue or something resolved during recent changes(I did a rebase recently).

[sorenlouv]

Just one comment about CODEOWNERS file

[sorenlouv]

Can't we simply use the existing viewer? Also, what do you expect to happen if unauthorizedUser is used to call an endpoint that viewer is allowed to access?

[viduni94]

Hey @arturoliduena
You might not be able to use the viewer role for unauthorizedUser in this manner for DA tests. It was done for serverless this way, because we force the user to be unauthorized by not passing the cookie header.

https://github.com/elastic/kibana/pull/205210/files#diff-febc7c89d3de714505db725fb9c264974eea6a5b29542b8782ebb0bab98a92d7R219

However, since the cookie is always included in makeInternalApiRequest, this user will be a viewer with authorization.

[arturoliduena]

Hey @sorenlouv and @viduni94,
We can simulate an unauthorized user by omitting credentials entirely with a function like makeInternalApiRequestWithoutCredentials:

  function makeInternalApiRequest(role: string) {
    return async <TEndpoint extends InternalEndpoint<APIEndpoint>>(
      options: Options<TEndpoint>
    ): Promise<SupertestReturnType<TEndpoint>> => {
      const headers: Record<string, string> = {
        ...samlAuth.getInternalRequestHeader(),
        ...(await samlAuth.getM2MApiCookieCredentialsWithRoleScope(role)),
      };

      return makeApiRequest({
        options,
        headers,
      });
    };
  }

  function makeInternalApiRequestWithoutCredentials() {
    return async <TEndpoint extends InternalEndpoint<APIEndpoint>>(
      options: Options<TEndpoint>
    ): Promise<SupertestReturnType<TEndpoint>> => {
      const headers: Record<string, string> = {
        ...samlAuth.getInternalRequestHeader(),
      };

      return makeApiRequest({
        options,
        headers,
      });
    };
  }

However, the problem with this approach is that omitting credentials will result in a 401 Unauthorized response instead of a 403 Forbidden. This doesn’t align with the intention of the test, which is to verify whether a user with insufficient permissions can access specific endpoints (DELETE, POST, PUT).

To properly simulate this, the user needs to be authenticated (avoiding 401 Unauthorized) but lack the necessary permissions, resulting in a 403 Forbidden. This is why I think it makes sense to use the viewer role here.

I agree with Søren that adding an additional unauthorizedUser role like this:

unauthorizedUser: observabilityAIAssistantApiClient.makeInternalApiRequest('viewer')

is unnecessary. Instead, we can simply use the existing viewer role:

viewer: observabilityAIAssistantApiClient.makeInternalApiRequest('viewer')

This keeps the implementation clean and ensures that we’re testing for insufficient permissions correctly. Let me know if you agree

[sorenlouv]

If I read that correctly, you'll authenticate as the viewer user. That sounds good to me.

[viduni94]

What I meant is not to have a user without credentials, but to have a user with a role that has insufficient privileges to access the AI Assistant APIs.

If the viewer role fits that criteria, I'm good with this.

[sorenlouv]

It just seems odd to have a user called unauthorizedUser. Because even with just the viewer role there are APIs they are authorised to access.

[sorenlouv]

fyi: @dgieselaar is extracting the API client in https://github.com/elastic/kibana/pull/204309/files#diff-cb59199a241f60514e9ba52c57b587929f6be6bcedb24c5a138d5ba72184fe85.

I don't think this is a blocker but eventually we should use that implementation so we don't have multiple. Afaict we now have three API clients:

• https://github.com/elastic/kibana/blob/main/x-pack/test/observability_ai_assistant_api_integration/common/observability_ai_assistant_api_client.ts
• https://github.com/elastic/kibana/blob/main/x-pack/test_serverless/api_integration/test_suites/observability/ai_assistant/common/observability_ai_assistant_api_client.ts

[neptunian]

We can be replace skipMKI with failsOnMKI if we want to be more descriptive that we aren't wanting to skip but that its failing so we have to skip.

[arturoliduena]

done!

[arturoliduena]

@neptunian, @sorenlouv and @viduni94
Here’s an update:
After enabling the ml test service, I continued with the migration of the knowledge_base tests.

kibana/x-pack/test/api_integration/deployment_agnostic/services/deployment_agnostic_services.ts

Line 15 in e04b200

export const deploymentAgnosticServices = _.pick(apiIntegrationServices, [

Migrated to Deployment-Agnostic:

• conversations.spec.ts
• connectors.spec.ts
• chat.spec.ts
• complete.spec.ts
• elasticsearch.spec.ts
• public_complete.spec.ts
• alerts.spec.ts
• knowledge_base_setup.spec.ts
• knowledge_base_status.spec.ts
• knowledge_base.spec.ts
• summarize.ts

Pending:

• Migrate knowledge_base_user_instructions.spec.ts
• Clean up all duplicated code

Issue with knowledge_base_user_instructions.spec.ts:

there is a difference between stateful and serverless environments.

In the Stateful Environment:

• We create users (e.g., editor and secondary_editor) and their respective user_instructions.
• The test ensures:
  • User A can access their own user_instructions and the public ones.
  • User B can do the same for their user_instructions.

Code reference:

kibana/x-pack/test/observability_ai_assistant_api_integration/tests/knowledge_base/knowledge_base_user_instructions.spec.ts

Lines 59 to 90 in 6d49348

	describe('when creating private and public user instructions', () => {
	before(async () => {
	await clearKnowledgeBase(es);
	const promises = [
	{
	username: 'editor' as const,
	isPublic: true,
	},
	{
	username: 'editor' as const,
	isPublic: false,
	},
	{
	username: 'secondary_editor' as const,
	isPublic: true,
	},
	{
	username: 'secondary_editor' as const,
	isPublic: false,
	},
	].map(async ({ username, isPublic }) => {
	const visibility = isPublic ? 'Public' : 'Private';
	await getScopedApiClientForUsername(username)({
	endpoint: 'PUT /internal/observability_ai_assistant/kb/user_instructions',
	params: {
	body: {
	id: `${visibility.toLowerCase()}-doc-from-${username}`,
	text: `${visibility} user instruction from "${username}"`,
	public: isPublic,
	},

In the Serverless Environment:

• We rely on roles (editor and admin) rather than user creation.
• The same functionality is tested, but achieved differently due to the lack of specific user creation.

Code reference:

kibana/x-pack/test_serverless/api_integration/test_suites/observability/ai_assistant/tests/knowledge_base/knowledge_base_user_instructions.spec.ts

Lines 72 to 98 in 6d49348

	describe('when creating private and public user instructions', () => {
	before(async () => {
	await clearKnowledgeBase(es);
	const promises = [
	{
	username: 'editor' as const,
	isPublic: true,
	},
	{
	username: 'editor' as const,
	isPublic: false,
	},
	{
	username: 'secondary_editor' as const,
	isPublic: true,
	},
	{
	username: 'secondary_editor' as const,
	isPublic: false,
	},
	].map(async ({ username, isPublic }) => {
	const visibility = isPublic ? 'Public' : 'Private';
	const user = username === 'editor' ? 'slsEditor' : 'slsAdmin';
	await observabilityAIAssistantAPIClient[user]({
	endpoint: 'PUT /internal/observability_ai_assistant/kb/user_instructions',

Question:

Should we migrate these tests to deployment-agnostic in a similar manner to how it is implemented in the serverless environment, or should we maintain them as separate implementations between stateful and serverless environments?

[neptunian]

Should we migrate these tests to deployment-agnostic in a similar manner to how it is implemented in the serverless environment, or should we maintain them as separate implementations between stateful and serverless environments?

@arturoliduena Thanks for bringing that question up. From what I've read here, my understanding is we should use the deployment agnostic samlAuth in the client (which you've done) and not create these users using basic auth as we currently are in stateful. Since the roles exist in stateful, they should be able to adopt the serverless approach and use the roles instead which should be tied to separate users. So yes, assuming this is a simple implementation using serverless-like roles, I would say we can make them deployment agnostic.

[elasticmachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: d2028d2
• Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-205194-d2028d2a6987

Failed CI Steps

• FTR Configs #63

Test Failures

• [job] [logs] FTR Configs #63 / Actions and Triggers app Maintenance Windows Maintenance windows table paginates maintenance windows correctly

Metrics [docs]

✅ unchanged

History

• 💚 Build #266441 succeeded 4c5743461dd944e8b88bd9bdf93947091431607c
• 💚 Build #265961 succeeded abd18f01c1bec6a2b2d664086f1b1165ce97537b
• 💚 Build #265903 succeeded 9e373057e8b3f85ef37cde7084305b45b3809016
• 💔 Build #265722 failed fa9e3cc4419b96a9435afdc7068971d9cc99cdab
• 💛 Build #265515 was flaky 0321c30c97b8264ad79e8ddc7d8e1401d62e3164

[kibanamachine]

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12755763835

[kibanamachine]

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12755763869

[kibanamachine]

💔 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 205194

Questions ?

Please refer to the Backport tool documentation

[kibanamachine]

💔 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 205194

Questions ?

Please refer to the Backport tool documentation

[arturoliduena]

💚 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