Skip to content

Commit

Permalink
[ML] New Platform server shim: update annotation routes to use new pl…
Browse files Browse the repository at this point in the history
…atform router (#57067) (#57361)

* update annotation routes to use NP router

* create ts file for feature check

* update schema to allow null value for earliest/latestMs

* update annotations model test

* use NP security plugin to access user info
  • Loading branch information
alvarezmelissa87 authored Feb 11, 2020
1 parent f5251bb commit 6797a0d
Show file tree
Hide file tree
Showing 11 changed files with 254 additions and 114 deletions.
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/ml/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const ml = (kibana: any) => {
const { usageCollection, cloud, home } = kbnServer.newPlatform.setup.plugins;
const plugins = {
elasticsearch: server.plugins.elasticsearch, // legacy
security: server.plugins.security,
security: server.newPlatform.setup.plugins.security,
xpackMain: server.plugins.xpack_main,
spaces: server.plugins.spaces,
home,
Expand Down
11 changes: 11 additions & 0 deletions x-pack/legacy/plugins/ml/server/lib/check_annotations/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { IScopedClusterClient } from 'src/core/server';

export function isAnnotationsFeatureAvailable(
callAsCurrentUser: IScopedClusterClient['callAsCurrentUser']
): boolean;
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,20 @@ import {
// - ML_ANNOTATIONS_INDEX_PATTERN index is present
// - ML_ANNOTATIONS_INDEX_ALIAS_READ alias is present
// - ML_ANNOTATIONS_INDEX_ALIAS_WRITE alias is present
export async function isAnnotationsFeatureAvailable(callWithRequest) {
export async function isAnnotationsFeatureAvailable(callAsCurrentUser) {
try {
const indexParams = { index: ML_ANNOTATIONS_INDEX_PATTERN };

const annotationsIndexExists = await callWithRequest('indices.exists', indexParams);
const annotationsIndexExists = await callAsCurrentUser('indices.exists', indexParams);
if (!annotationsIndexExists) return false;

const annotationsReadAliasExists = await callWithRequest('indices.existsAlias', {
const annotationsReadAliasExists = await callAsCurrentUser('indices.existsAlias', {
name: ML_ANNOTATIONS_INDEX_ALIAS_READ,
});

if (!annotationsReadAliasExists) return false;

const annotationsWriteAliasExists = await callWithRequest('indices.existsAlias', {
const annotationsWriteAliasExists = await callAsCurrentUser('indices.existsAlias', {
name: ML_ANNOTATIONS_INDEX_ALIAS_WRITE,
});
if (!annotationsWriteAliasExists) return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import getAnnotationsRequestMock from './__mocks__/get_annotations_request.json';
import getAnnotationsResponseMock from './__mocks__/get_annotations_response.json';
import { RequestHandlerContext } from 'src/core/server';

import { ANNOTATION_TYPE } from '../../../common/constants/annotations';
import { ML_ANNOTATIONS_INDEX_ALIAS_WRITE } from '../../../common/constants/index_patterns';
Expand All @@ -19,23 +20,30 @@ const acknowledgedResponseMock = { acknowledged: true };
const jobIdMock = 'jobIdMock';

describe('annotation_service', () => {
let callWithRequestSpy: jest.Mock;
let callWithRequestSpy: any;

beforeEach(() => {
callWithRequestSpy = jest.fn((action: string) => {
switch (action) {
case 'delete':
case 'index':
return Promise.resolve(acknowledgedResponseMock);
case 'search':
return Promise.resolve(getAnnotationsResponseMock);
}
});
callWithRequestSpy = ({
ml: {
mlClient: {
callAsCurrentUser: jest.fn((action: string) => {
switch (action) {
case 'delete':
case 'index':
return Promise.resolve(acknowledgedResponseMock);
case 'search':
return Promise.resolve(getAnnotationsResponseMock);
}
}),
},
},
} as unknown) as RequestHandlerContext;
});

describe('deleteAnnotation()', () => {
it('should delete annotation', async done => {
const { deleteAnnotation } = annotationServiceProvider(callWithRequestSpy);
const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser;

const annotationMockId = 'mockId';
const deleteParamsMock: DeleteParams = {
Expand All @@ -46,8 +54,8 @@ describe('annotation_service', () => {

const response = await deleteAnnotation(annotationMockId);

expect(callWithRequestSpy.mock.calls[0][0]).toBe('delete');
expect(callWithRequestSpy.mock.calls[0][1]).toEqual(deleteParamsMock);
expect(mockFunct.mock.calls[0][0]).toBe('delete');
expect(mockFunct.mock.calls[0][1]).toEqual(deleteParamsMock);
expect(response).toBe(acknowledgedResponseMock);
done();
});
Expand All @@ -56,6 +64,7 @@ describe('annotation_service', () => {
describe('getAnnotation()', () => {
it('should get annotations for specific job', async done => {
const { getAnnotations } = annotationServiceProvider(callWithRequestSpy);
const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser;

const indexAnnotationArgsMock: IndexAnnotationArgs = {
jobIds: [jobIdMock],
Expand All @@ -66,8 +75,8 @@ describe('annotation_service', () => {

const response: GetResponse = await getAnnotations(indexAnnotationArgsMock);

expect(callWithRequestSpy.mock.calls[0][0]).toBe('search');
expect(callWithRequestSpy.mock.calls[0][1]).toEqual(getAnnotationsRequestMock);
expect(mockFunct.mock.calls[0][0]).toBe('search');
expect(mockFunct.mock.calls[0][1]).toEqual(getAnnotationsRequestMock);
expect(Object.keys(response.annotations)).toHaveLength(1);
expect(response.annotations[jobIdMock]).toHaveLength(2);
expect(isAnnotations(response.annotations[jobIdMock])).toBeTruthy();
Expand All @@ -81,9 +90,15 @@ describe('annotation_service', () => {
message: 'mock error message',
};

const callWithRequestSpyError = jest.fn(() => {
return Promise.resolve(mockEsError);
});
const callWithRequestSpyError = ({
ml: {
mlClient: {
callAsCurrentUser: jest.fn(() => {
return Promise.resolve(mockEsError);
}),
},
},
} as unknown) as RequestHandlerContext;

const { getAnnotations } = annotationServiceProvider(callWithRequestSpyError);

Expand All @@ -103,6 +118,7 @@ describe('annotation_service', () => {
describe('indexAnnotation()', () => {
it('should index annotation', async done => {
const { indexAnnotation } = annotationServiceProvider(callWithRequestSpy);
const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser;

const annotationMock: Annotation = {
annotation: 'Annotation text',
Expand All @@ -114,10 +130,10 @@ describe('annotation_service', () => {

const response = await indexAnnotation(annotationMock, usernameMock);

expect(callWithRequestSpy.mock.calls[0][0]).toBe('index');
expect(mockFunct.mock.calls[0][0]).toBe('index');

// test if the annotation has been correctly augmented
const indexParamsCheck = callWithRequestSpy.mock.calls[0][1];
const indexParamsCheck = mockFunct.mock.calls[0][1];
const annotation = indexParamsCheck.body;
expect(annotation.create_username).toBe(usernameMock);
expect(annotation.modified_username).toBe(usernameMock);
Expand All @@ -130,6 +146,7 @@ describe('annotation_service', () => {

it('should remove ._id and .key before updating annotation', async done => {
const { indexAnnotation } = annotationServiceProvider(callWithRequestSpy);
const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser;

const annotationMock: Annotation = {
_id: 'mockId',
Expand All @@ -143,10 +160,10 @@ describe('annotation_service', () => {

const response = await indexAnnotation(annotationMock, usernameMock);

expect(callWithRequestSpy.mock.calls[0][0]).toBe('index');
expect(mockFunct.mock.calls[0][0]).toBe('index');

// test if the annotation has been correctly augmented
const indexParamsCheck = callWithRequestSpy.mock.calls[0][1];
const indexParamsCheck = mockFunct.mock.calls[0][1];
const annotation = indexParamsCheck.body;
expect(annotation.create_username).toBe(usernameMock);
expect(annotation.modified_username).toBe(usernameMock);
Expand All @@ -161,6 +178,7 @@ describe('annotation_service', () => {

it('should update annotation text and the username for modified_username', async done => {
const { getAnnotations, indexAnnotation } = annotationServiceProvider(callWithRequestSpy);
const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser;

const indexAnnotationArgsMock: IndexAnnotationArgs = {
jobIds: [jobIdMock],
Expand All @@ -184,9 +202,9 @@ describe('annotation_service', () => {

await indexAnnotation(annotation, modifiedUsernameMock);

expect(callWithRequestSpy.mock.calls[1][0]).toBe('index');
expect(mockFunct.mock.calls[1][0]).toBe('index');
// test if the annotation has been correctly updated
const indexParamsCheck = callWithRequestSpy.mock.calls[1][1];
const indexParamsCheck = mockFunct.mock.calls[1][1];
const modifiedAnnotation = indexParamsCheck.body;
expect(modifiedAnnotation.annotation).toBe(modifiedAnnotationText);
expect(modifiedAnnotation.create_username).toBe(originalUsernameMock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import Boom from 'boom';
import _ from 'lodash';
import { RequestHandlerContext } from 'src/core/server';

import { ANNOTATION_TYPE } from '../../../common/constants/annotations';
import {
Expand Down Expand Up @@ -67,7 +68,8 @@ export type callWithRequestType = (
params: annotationProviderParams
) => Promise<any>;

export function annotationProvider(callWithRequest: callWithRequestType) {
export function annotationProvider(context: RequestHandlerContext) {
const callAsCurrentUser = context.ml!.mlClient.callAsCurrentUser;
async function indexAnnotation(annotation: Annotation, username: string) {
if (isAnnotation(annotation) === false) {
// No need to translate, this will not be exposed in the UI.
Expand All @@ -94,7 +96,7 @@ export function annotationProvider(callWithRequest: callWithRequestType) {
delete params.body.key;
}

return await callWithRequest('index', params);
return await callAsCurrentUser('index', params);
}

async function getAnnotations({
Expand Down Expand Up @@ -213,7 +215,7 @@ export function annotationProvider(callWithRequest: callWithRequestType) {
};

try {
const resp = await callWithRequest('search', params);
const resp = await callAsCurrentUser('search', params);

if (resp.error !== undefined && resp.message !== undefined) {
// No need to translate, this will not be exposed in the UI.
Expand Down Expand Up @@ -252,7 +254,7 @@ export function annotationProvider(callWithRequest: callWithRequestType) {
refresh: 'wait_for',
};

return await callWithRequest('delete', param);
return await callAsCurrentUser('delete', param);
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { annotationProvider, callWithRequestType } from './annotation';
import { RequestHandlerContext } from 'src/core/server';
import { annotationProvider } from './annotation';

export function annotationServiceProvider(callWithRequest: callWithRequestType) {
export function annotationServiceProvider(context: RequestHandlerContext) {
return {
...annotationProvider(callWithRequest),
...annotationProvider(context),
};
}
30 changes: 30 additions & 0 deletions x-pack/legacy/plugins/ml/server/new_platform/annotations_schema.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { schema } from '@kbn/config-schema';

export const indexAnnotationSchema = {
timestamp: schema.number(),
end_timestamp: schema.number(),
annotation: schema.string(),
job_id: schema.string(),
type: schema.string(),
create_time: schema.maybe(schema.number()),
create_username: schema.maybe(schema.string()),
modified_time: schema.maybe(schema.number()),
modified_username: schema.maybe(schema.string()),
_id: schema.maybe(schema.string()),
key: schema.maybe(schema.string()),
};

export const getAnnotationsSchema = {
jobIds: schema.arrayOf(schema.string()),
earliestMs: schema.oneOf([schema.nullable(schema.number()), schema.maybe(schema.number())]),
latestMs: schema.oneOf([schema.nullable(schema.number()), schema.maybe(schema.number())]),
maxAnnotations: schema.number(),
};

export const deleteAnnotationSchema = { annotationId: schema.string() };
2 changes: 2 additions & 0 deletions x-pack/legacy/plugins/ml/server/new_platform/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export interface RouteInitialization {
xpackMainPlugin: MlXpackMainPlugin;
savedObjects?: SavedObjectsLegacyService;
spacesPlugin: any;
securityPlugin: any;
cloud?: CloudSetup;
}
export interface UsageInitialization {
Expand Down Expand Up @@ -212,6 +213,7 @@ export class Plugin {
elasticsearchService: core.elasticsearch,
xpackMainPlugin: plugins.xpackMain,
spacesPlugin: plugins.spaces,
securityPlugin: plugins.security,
};

const extendedRouteInitializationDeps: RouteInitialization = {
Expand Down
78 changes: 0 additions & 78 deletions x-pack/legacy/plugins/ml/server/routes/annotations.js

This file was deleted.

Loading

0 comments on commit 6797a0d

Please sign in to comment.