-
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
[Obs AI Assistant] Serverless API integration tests #192219
Changes from 7 commits
fa8d99f
ec519e1
e23f82a
c9c90ad
46a408d
f8abcd4
ed0def8
373958d
c7a80fb
19bc673
6ed95af
1559e5b
23ac89e
ff8dc82
5f25e28
880e67c
c41142c
34117fc
67cf3ba
2eae7c0
e57eef9
6fbc13f
94ebcc8
d8b3976
2c39661
28e523c
925381e
48e7ff7
c3b32eb
5bf0f8f
e0289e3
2e8ff43
299be68
bf70156
7bacda2
cdd7901
5de96ec
4af20ab
7b0ad92
672ce27
2d0e54d
835af9f
b7a9e64
4284818
e63adfc
c518762
55df2f2
c237b6d
3a6d421
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,8 @@ export interface RecalledEntry { | |
function isModelMissingOrUnavailableError(error: Error) { | ||
return ( | ||
error instanceof errors.ResponseError && | ||
error.body && | ||
error.body.error && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it this a type error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, if I recall, I was getting some errors in serverless that didn't have these props. I updated it to use optional chaining. |
||
(error.body.error.type === 'resource_not_found_exception' || | ||
error.body.error.type === 'status_exception') | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { ToolingLog } from '@kbn/tooling-log'; | ||
import type { | ||
InternalRequestHeader, | ||
RoleCredentials, | ||
SupertestWithoutAuthProviderType, | ||
} from '../../../../../shared/services'; | ||
|
||
export async function deleteActionConnector({ | ||
supertest, | ||
connectorId, | ||
log, | ||
roleAuthc, | ||
internalReqHeader, | ||
}: { | ||
supertest: SupertestWithoutAuthProviderType; | ||
connectorId: string; | ||
log: ToolingLog; | ||
roleAuthc: RoleCredentials; | ||
internalReqHeader: InternalRequestHeader; | ||
}) { | ||
try { | ||
await supertest | ||
.delete(`/api/actions/connector/${connectorId}`) | ||
.set(roleAuthc.apiKeyHeader) | ||
.set(internalReqHeader) | ||
Comment on lines
+31
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would it be possible to move the authentication part to a service? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes but I will likely be removing them all in favor of cookie auth. See #192711. I am hoping to do it as part of that issue. The cookie auth was made available recently in #192727. We should only be using api keys for public apis. It should also allow us to use different users where as with the api key its always using the same user despite passing different roles. |
||
.expect(204); | ||
} catch (e) { | ||
log.error(`Failed to delete action connector with id ${connectorId} due to: ${e}`); | ||
throw e; | ||
} | ||
} | ||
|
||
export async function createProxyActionConnector({ | ||
log, | ||
supertest, | ||
port, | ||
roleAuthc, | ||
internalReqHeader, | ||
}: { | ||
log: ToolingLog; | ||
supertest: SupertestWithoutAuthProviderType; | ||
port: number; | ||
roleAuthc: RoleCredentials; | ||
internalReqHeader: InternalRequestHeader; | ||
}) { | ||
try { | ||
const res = await supertest | ||
.post('/api/actions/connector') | ||
.set(roleAuthc.apiKeyHeader) | ||
.set(internalReqHeader) | ||
.send({ | ||
name: 'OpenAI Proxy', | ||
connector_type_id: '.gen-ai', | ||
config: { | ||
apiProvider: 'OpenAI', | ||
apiUrl: `http://localhost:${port}`, | ||
}, | ||
secrets: { | ||
apiKey: 'my-api-key', | ||
}, | ||
}) | ||
.expect(200); | ||
|
||
const connectorId = res.body.id as string; | ||
return connectorId; | ||
} catch (e) { | ||
log.error(`Failed to create action connector due to: ${e}`); | ||
throw e; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { GenericFtrProviderContext } from '@kbn/test'; | ||
import { InheritedServices, InheritedFtrProviderContext } from '../../../../services'; | ||
import { ObservabilityAIAssistantApiClient } from './observability_ai_assistant_api_client'; | ||
|
||
export type ObservabilityAIAssistantServices = InheritedServices & { | ||
observabilityAIAssistantAPIClient: ( | ||
context: InheritedFtrProviderContext | ||
) => Promise<ObservabilityAIAssistantApiClient>; | ||
}; | ||
|
||
export type FtrProviderContext = GenericFtrProviderContext<ObservabilityAIAssistantServices, {}>; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,195 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import type { | ||
APIReturnType, | ||
ObservabilityAIAssistantAPIClientRequestParamsOf, | ||
ObservabilityAIAssistantAPIEndpoint, | ||
} from '@kbn/observability-ai-assistant-plugin/public'; | ||
import { formatRequest } from '@kbn/server-route-repository'; | ||
import supertest from 'supertest'; | ||
import { Subtract } from 'utility-types'; | ||
import { format } from 'url'; | ||
import { Config, kbnTestConfig, kibanaTestSuperuserServerless } from '@kbn/test'; | ||
import { InheritedFtrProviderContext } from '../../../../services'; | ||
import type { InternalRequestHeader, RoleCredentials } from '../../../../../shared/services'; | ||
|
||
export function getObservabilityAIAssistantApiClient({ | ||
svlSharedConfig, | ||
}: { | ||
svlSharedConfig: Config; | ||
}) { | ||
const kibanaServer = svlSharedConfig.get('servers.kibana'); | ||
const cAuthorities = svlSharedConfig.get('servers.kibana.certificateAuthorities'); | ||
|
||
const username = kbnTestConfig.getUrlParts(kibanaTestSuperuserServerless).username; | ||
const password = kbnTestConfig.getUrlParts(kibanaTestSuperuserServerless).password; | ||
|
||
const url = format({ | ||
...kibanaServer, | ||
auth: `${username}:${password}`, | ||
}); | ||
|
||
return createObservabilityAIAssistantApiClient(supertest.agent(url, { ca: cAuthorities })); | ||
} | ||
|
||
type ObservabilityAIAssistantApiClientKey = 'slsUser'; | ||
export type ObservabilityAIAssistantApiClient = Record< | ||
ObservabilityAIAssistantApiClientKey, | ||
Awaited<ReturnType<typeof getObservabilityAIAssistantApiClient>> | ||
>; | ||
export function createObservabilityAIAssistantApiClient(st: supertest.Agent) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any way we can move this out into a separate package that can be shared across environments? I understand there's some serverless-specific stuff in here, but I'd guess there's also a lot of things that can be shared. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea for sure. I'd like to do that as part of #192718. I've updated it to call this out specifically. |
||
return <TEndpoint extends ObservabilityAIAssistantAPIEndpoint>( | ||
options: { | ||
type?: 'form-data'; | ||
endpoint: TEndpoint; | ||
roleAuthc: RoleCredentials; | ||
internalReqHeader: InternalRequestHeader; | ||
} & ObservabilityAIAssistantAPIClientRequestParamsOf<TEndpoint> & { | ||
params?: { query?: { _inspect?: boolean } }; | ||
} | ||
): SupertestReturnType<TEndpoint> => { | ||
const { endpoint, type, roleAuthc, internalReqHeader } = options; | ||
|
||
const params = 'params' in options ? (options.params as Record<string, any>) : {}; | ||
|
||
const { method, pathname, version } = formatRequest(endpoint, params.path); | ||
const url = format({ pathname, query: params?.query }); | ||
|
||
const headers: Record<string, string> = { ...internalReqHeader, ...roleAuthc.apiKeyHeader }; | ||
|
||
if (version) { | ||
headers['Elastic-Api-Version'] = version; | ||
} | ||
|
||
let res: supertest.Test; | ||
if (type === 'form-data') { | ||
const fields: Array<[string, any]> = Object.entries(params.body); | ||
const formDataRequest = st[method](url) | ||
.set(headers) | ||
.set('Content-type', 'multipart/form-data'); | ||
|
||
for (const field of fields) { | ||
void formDataRequest.field(field[0], field[1]); | ||
} | ||
|
||
res = formDataRequest; | ||
} else if (params.body) { | ||
res = st[method](url).send(params.body).set(headers); | ||
} else { | ||
res = st[method](url).set(headers); | ||
} | ||
|
||
return res as unknown as SupertestReturnType<TEndpoint>; | ||
}; | ||
} | ||
|
||
export type ObservabilityAIAssistantAPIClient = ReturnType< | ||
typeof createObservabilityAIAssistantApiClient | ||
>; | ||
|
||
type WithoutPromise<T extends Promise<any>> = Subtract<T, Promise<any>>; | ||
|
||
// this is a little intense, but without it, method overrides are lost | ||
// e.g., { | ||
// end(one:string) | ||
// end(one:string, two:string) | ||
// } | ||
// would lose the first signature. This keeps up to eight signatures. | ||
type OverloadedParameters<T> = T extends { | ||
(...args: infer A1): any; | ||
(...args: infer A2): any; | ||
(...args: infer A3): any; | ||
(...args: infer A4): any; | ||
(...args: infer A5): any; | ||
(...args: infer A6): any; | ||
(...args: infer A7): any; | ||
(...args: infer A8): any; | ||
} | ||
? A1 | A2 | A3 | A4 | A5 | A6 | A7 | A8 | ||
: T extends { | ||
(...args: infer A1): any; | ||
(...args: infer A2): any; | ||
(...args: infer A3): any; | ||
(...args: infer A4): any; | ||
(...args: infer A5): any; | ||
(...args: infer A6): any; | ||
(...args: infer A7): any; | ||
} | ||
? A1 | A2 | A3 | A4 | A5 | A6 | A7 | ||
: T extends { | ||
(...args: infer A1): any; | ||
(...args: infer A2): any; | ||
(...args: infer A3): any; | ||
(...args: infer A4): any; | ||
(...args: infer A5): any; | ||
(...args: infer A6): any; | ||
} | ||
? A1 | A2 | A3 | A4 | A5 | A6 | ||
: T extends { | ||
(...args: infer A1): any; | ||
(...args: infer A2): any; | ||
(...args: infer A3): any; | ||
(...args: infer A4): any; | ||
(...args: infer A5): any; | ||
} | ||
? A1 | A2 | A3 | A4 | A5 | ||
: T extends { | ||
(...args: infer A1): any; | ||
(...args: infer A2): any; | ||
(...args: infer A3): any; | ||
(...args: infer A4): any; | ||
} | ||
? A1 | A2 | A3 | A4 | ||
: T extends { | ||
(...args: infer A1): any; | ||
(...args: infer A2): any; | ||
(...args: infer A3): any; | ||
} | ||
? A1 | A2 | A3 | ||
: T extends { | ||
(...args: infer A1): any; | ||
(...args: infer A2): any; | ||
} | ||
? A1 | A2 | ||
: T extends (...args: infer A) => any | ||
? A | ||
: any; | ||
|
||
type OverrideReturnType<T extends (...args: any[]) => any, TNextReturnType> = ( | ||
...args: OverloadedParameters<T> | ||
) => WithoutPromise<ReturnType<T>> & TNextReturnType; | ||
|
||
type OverwriteThisMethods<T extends Record<string, any>, TNextReturnType> = TNextReturnType & { | ||
[key in keyof T]: T[key] extends (...args: infer TArgs) => infer TReturnType | ||
? TReturnType extends Promise<any> | ||
? OverrideReturnType<T[key], TNextReturnType> | ||
: (...args: TArgs) => TReturnType | ||
: T[key]; | ||
}; | ||
|
||
export type SupertestReturnType<TEndpoint extends ObservabilityAIAssistantAPIEndpoint> = | ||
OverwriteThisMethods< | ||
WithoutPromise<supertest.Test>, | ||
Promise<{ | ||
text: string; | ||
status: number; | ||
body: APIReturnType<TEndpoint>; | ||
}> | ||
>; | ||
|
||
export async function getObservabilityAIAssistantApiClientService({ | ||
getService, | ||
}: InheritedFtrProviderContext): Promise<ObservabilityAIAssistantApiClient> { | ||
const svlSharedConfig = getService('config'); | ||
|
||
return { | ||
slsUser: await getObservabilityAIAssistantApiClient({ | ||
svlSharedConfig, | ||
}), | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { SUPPORTED_TRAINED_MODELS } from '@kbn/test-suites-xpack/functional/services/ml/api'; | ||
import { createTestConfig } from '../../../config.base'; | ||
import { ObservabilityAIAssistantServices } from './common/ftr_provider_context'; | ||
import { services as inheritedServices } from '../../../services'; | ||
import { getObservabilityAIAssistantApiClientService } from './common/observability_ai_assistant_api_client'; | ||
|
||
export const services: ObservabilityAIAssistantServices = { | ||
...inheritedServices, | ||
observabilityAIAssistantAPIClient: getObservabilityAIAssistantApiClientService, | ||
}; | ||
|
||
export default createTestConfig({ | ||
serverlessProject: 'oblt', | ||
testFiles: [require.resolve('./tests')], | ||
junit: { | ||
reportName: 'Observability AI Assistant API Integration tests', | ||
}, | ||
suiteTags: { exclude: ['skipSvlOblt'] }, | ||
services, | ||
|
||
// include settings from project controller | ||
// https://github.com/elastic/project-controller/blob/main/internal/project/observability/config/elasticsearch.yml | ||
esServerArgs: ['xpack.ml.dfa.enabled=false'], | ||
kbnServerArgs: [ | ||
`--xpack.observabilityAIAssistant.modelId=${SUPPORTED_TRAINED_MODELS.TINY_ELSER.name}`, | ||
], | ||
}); |
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.
BufferFlushEvent
can be returned as a streaming chat response event in cloudThere 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.
Yes but it's intentionally not here to not overcomplicate the
StreamingChatResponseEvent
typeThere 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 only added it due to causing a type error in complete.spec.ts due to filtering out
BufferFlushEvent
s appearing in the data. Is it okay to leave it?