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

[ML] job saved objects initialization #82639

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
01b7621
[ML] job saved objects initialization
jgowdyelastic Nov 3, 2020
f934953
fixing job count logic
jgowdyelastic Nov 3, 2020
46d0081
adding missing files
jgowdyelastic Nov 3, 2020
a6b230c
attempting to fix build crash
jgowdyelastic Nov 3, 2020
d80b4a7
fixing kibana.json
jgowdyelastic Nov 3, 2020
eb8a4f6
changes based on review
jgowdyelastic Nov 4, 2020
27efbe3
removing accidentally added export
jgowdyelastic Nov 5, 2020
9388579
Merge branch 'master' into job-saved-object-initialization-2
kibanamachine Nov 5, 2020
43cd5f6
adding intialization promise
jgowdyelastic Nov 5, 2020
84fa0f9
use finally so errors dont stop initialization
jgowdyelastic Nov 6, 2020
0b1bbf7
Merge branch 'master' into job-saved-object-initialization-2
kibanamachine Nov 6, 2020
8c6751f
function rename
jgowdyelastic Nov 6, 2020
b9d45ff
removing duplicate header
jgowdyelastic Nov 6, 2020
a959ee7
adding job initialization count to log message
jgowdyelastic Nov 6, 2020
d2800ab
adding error to log message
jgowdyelastic Nov 6, 2020
c323370
Merge branch 'master' into job-saved-object-initialization-2
kibanamachine Nov 9, 2020
63769d4
moving initialization file
jgowdyelastic Nov 9, 2020
fb6a991
moving intialization file back again to fix git stash issues
jgowdyelastic Nov 9, 2020
216a9d2
Merge branch 'master' into job-saved-object-initialization-2
kibanamachine Nov 9, 2020
3701886
removing .kibana index search
jgowdyelastic Nov 10, 2020
f7f1e6a
Merge branch 'master' into job-saved-object-initialization-2
kibanamachine Nov 10, 2020
5c3d751
creating internal saved object client
jgowdyelastic Nov 10, 2020
b839622
code clean up
jgowdyelastic Nov 10, 2020
9324bdd
removing commented code
jgowdyelastic Nov 10, 2020
b81496e
adding check for spaces enabled
jgowdyelastic Nov 10, 2020
51bbb2b
adding ids to saved objects
jgowdyelastic Nov 10, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion x-pack/plugins/ml/common/types/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { KibanaRequest } from 'kibana/server';
import { PLUGIN_ID } from '../constants/app';
import { ML_SAVED_OBJECT_TYPE } from './saved_objects';

export const apmUserMlCapabilities = {
canGetJobs: false,
Expand Down Expand Up @@ -78,7 +79,13 @@ export function getPluginPrivileges() {
const adminMlCapabilitiesKeys = Object.keys(adminMlCapabilities);
const allMlCapabilitiesKeys = [...adminMlCapabilitiesKeys, ...userMlCapabilitiesKeys];
// TODO: include ML in base privileges for the `8.0` release: https://github.com/elastic/kibana/issues/71422
const savedObjects = ['index-pattern', 'dashboard', 'search', 'visualization', 'ml-job'];
const savedObjects = [
'index-pattern',
'dashboard',
'search',
'visualization',
ML_SAVED_OBJECT_TYPE,
];
const privilege = {
app: [PLUGIN_ID, 'kibana'],
excludeFromBasePrivileges: true,
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/ml/common/types/saved_objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
*/

export type JobType = 'anomaly-detector' | 'data-frame-analytics';
export const ML_SAVED_OBJECT_TYPE = 'ml-job';
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import { HttpService } from '../http_service';

import { basePath } from './index';
import { JobType } from '../../../../common/types/saved_objects';

export const savedObjectsApiProvider = (httpService: HttpService) => ({
jobsSpaces() {
Expand All @@ -17,4 +18,20 @@ export const savedObjectsApiProvider = (httpService: HttpService) => ({
method: 'GET',
});
},
assignJobToSpace(jobType: JobType, jobIds: string[], spaces: string[]) {
const body = JSON.stringify({ jobType, jobIds, spaces });
return httpService.http<any>({
path: `${basePath()}/saved_objects/assign_job_to_space`,
method: 'POST',
body,
});
},
removeJobFromSpace(jobType: JobType, jobIds: string[], spaces: string[]) {
const body = JSON.stringify({ jobType, jobIds, spaces });
return httpService.http<any>({
path: `${basePath()}/saved_objects/remove_job_from_space`,
method: 'POST',
body,
});
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { KibanaRequest } from 'kibana/server';
import type { MlClient } from '../../lib/ml_client';
import { mlLog } from '../../client/log';
import { mlLog } from '../../lib/log';
import {
MlCapabilities,
adminMlCapabilities,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/ml/server/lib/capabilities/upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import type { MlClient } from '../../lib/ml_client';
import { mlLog } from '../../client/log';
import { mlLog } from '../../lib/log';

export function upgradeCheckProvider(mlClient: MlClient) {
async function isUpgradeInProgress(): Promise<boolean> {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/ml/server/lib/check_annotations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { IScopedClusterClient } from 'kibana/server';
import { mlLog } from '../../client/log';
import { mlLog } from '../../lib/log';

import {
ML_ANNOTATIONS_INDEX_ALIAS_READ,
Expand Down
26 changes: 23 additions & 3 deletions x-pack/plugins/ml/server/lib/route_guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
RequestHandler,
SavedObjectsClientContract,
} from 'kibana/server';
import { SpacesPluginSetup } from '../../../spaces/server';

import { jobSavedObjectServiceFactory, JobSavedObjectService } from '../saved_objects';
import { MlLicense } from '../../common/license';
Expand All @@ -28,14 +29,27 @@ type Handler = (handlerParams: {
}) => ReturnType<RequestHandler>;

type GetMlSavedObjectClient = (request: KibanaRequest) => SavedObjectsClientContract | null;
type GetInternalSavedObjectClient = () => SavedObjectsClientContract | null;

export class RouteGuard {
private _mlLicense: MlLicense;
private _getMlSavedObjectClient: GetMlSavedObjectClient;
private _getInternalSavedObjectClient: GetInternalSavedObjectClient;
private _spacesPlugin: SpacesPluginSetup | undefined;
private _isMlReady: () => Promise<void>;

constructor(mlLicense: MlLicense, getSavedObject: GetMlSavedObjectClient) {
constructor(
mlLicense: MlLicense,
getSavedObject: GetMlSavedObjectClient,
getInternalSavedObject: GetInternalSavedObjectClient,
spacesPlugin: SpacesPluginSetup | undefined,
isMlReady: () => Promise<void>
) {
this._mlLicense = mlLicense;
this._getMlSavedObjectClient = getSavedObject;
this._getInternalSavedObjectClient = getInternalSavedObject;
this._spacesPlugin = spacesPlugin;
this._isMlReady = isMlReady;
}

public fullLicenseAPIGuard(handler: Handler) {
Expand All @@ -56,13 +70,19 @@ export class RouteGuard {
}

const mlSavedObjectClient = this._getMlSavedObjectClient(request);
if (mlSavedObjectClient === null) {
const internalSavedObjectsClient = this._getInternalSavedObjectClient();
if (mlSavedObjectClient === null || internalSavedObjectsClient === null) {
return response.badRequest({
body: { message: 'saved object client has not been initialized' },
});
}

const jobSavedObjectService = jobSavedObjectServiceFactory(mlSavedObjectClient);
const jobSavedObjectService = jobSavedObjectServiceFactory(
mlSavedObjectClient,
internalSavedObjectsClient,
this._spacesPlugin !== undefined,
this._isMlReady
);
const client = context.core.elasticsearch.client;

return handler({
Expand Down
41 changes: 18 additions & 23 deletions x-pack/plugins/ml/server/lib/spaces_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,31 @@

import { Legacy } from 'kibana';
import { KibanaRequest } from 'kibana/server';
import { Space, SpacesPluginSetup } from '../../../spaces/server';
import { SpacesPluginSetup } from '../../../spaces/server';

export type RequestFacade = KibanaRequest | Legacy.Request;

interface GetActiveSpaceResponse {
valid: boolean;
space?: Space;
}

export function spacesUtilsProvider(spacesPlugin: SpacesPluginSetup, request: RequestFacade) {
async function activeSpace(): Promise<GetActiveSpaceResponse> {
try {
return {
valid: true,
space: await spacesPlugin.spacesService.getActiveSpace(request),
};
} catch (e) {
return {
valid: false,
};
export function spacesUtilsProvider(
spacesPlugin: SpacesPluginSetup | undefined,
request: RequestFacade
) {
async function isMlEnabledInSpace(): Promise<boolean> {
if (spacesPlugin === undefined) {
// if spaces is disabled force isMlEnabledInSpace to be true
return true;
}
const space = await spacesPlugin.spacesService.getActiveSpace(request);
return space.disabledFeatures.includes('ml') === false;
}

async function isMlEnabledInSpace(): Promise<boolean> {
const { valid, space } = await activeSpace();
if (valid === true && space !== undefined) {
return space.disabledFeatures.includes('ml') === false;
async function getAllSpaces(): Promise<string[] | null> {
if (spacesPlugin === undefined) {
return null;
}
return true;
const client = await spacesPlugin.spacesService.scopedClient(request);
const spaces = await client.getAll();
return spaces.map((s) => s.id);
}

return { isMlEnabledInSpace };
return { isMlEnabledInSpace, getAllSpaces };
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { cloneDeep, each, remove, sortBy, get } from 'lodash';

import { mlLog } from '../../client/log';
import { mlLog } from '../../lib/log';

import { INTERVALS } from './intervals';
import { singleSeriesCheckerFactory } from './single_series_checker';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* Bucket spans: 5m, 10m, 30m, 1h, 3h
*/

import { mlLog } from '../../client/log';
import { mlLog } from '../../lib/log';
import { INTERVALS, LONG_INTERVALS } from './intervals';

export function singleSeriesCheckerFactory({ asCurrentUser }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import {
prefixDatafeedId,
splitIndexPatternNames,
} from '../../../common/util/job_utils';
import { mlLog } from '../../client/log';
import { mlLog } from '../../lib/log';
import { calculateModelMemoryLimitProvider } from '../calculate_model_memory_limit';
import { fieldsServiceProvider } from '../fields_service';
import { jobServiceProvider } from '../job_service';
Expand Down
48 changes: 35 additions & 13 deletions x-pack/plugins/ml/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ import {
CapabilitiesStart,
IClusterClient,
SavedObjectsServiceStart,
SavedObjectsClientContract,
} from 'kibana/server';
import { DEFAULT_APP_CATEGORIES } from '../../../../src/core/server';
import { SpacesPluginSetup } from '../../spaces/server';
import { PluginsSetup, RouteInitialization } from './types';
import { PLUGIN_ID } from '../common/constants/app';
import { MlCapabilities } from '../common/types/capabilities';

import { initMlTelemetry } from './lib/telemetry';
import { initMlServerLog } from './client/log';
import { initMlServerLog } from './lib/log';
import { initSampleDataSets } from './lib/sample_data_sets';

import { annotationRoutes } from './routes/annotations';
Expand All @@ -50,7 +50,11 @@ import { getPluginPrivileges } from '../common/types/capabilities';
import { setupCapabilitiesSwitcher } from './lib/capabilities';
import { registerKibanaSettings } from './lib/register_settings';
import { trainedModelsRoutes } from './routes/trained_models';
import { setupSavedObjects } from './saved_objects';
import {
setupSavedObjects,
jobSavedObjectsInitializationFactory,
savedObjectClientsFactory,
} from './saved_objects';
import { RouteGuard } from './lib/route_guard';

export type MlPluginSetup = SharedServices;
Expand All @@ -63,14 +67,19 @@ export class MlServerPlugin implements Plugin<MlPluginSetup, MlPluginStart, Plug
private capabilities: CapabilitiesStart | null = null;
private clusterClient: IClusterClient | null = null;
private savedObjectsStart: SavedObjectsServiceStart | null = null;
private spacesPlugin: SpacesPluginSetup | undefined;
private isMlReady: Promise<void>;
private setMlReady: () => void = () => {};

constructor(ctx: PluginInitializerContext) {
this.log = ctx.logger.get();
this.version = ctx.env.packageInfo.branch;
this.mlLicense = new MlLicense();
this.isMlReady = new Promise((resolve) => (this.setMlReady = resolve));
}

public setup(coreSetup: CoreSetup, plugins: PluginsSetup): MlPluginSetup {
this.spacesPlugin = plugins.spaces;
const { admin, user, apmUser } = getPluginPrivileges();

plugins.features.registerKibanaFeature({
Expand Down Expand Up @@ -120,18 +129,19 @@ export class MlServerPlugin implements Plugin<MlPluginSetup, MlPluginStart, Plug
setupCapabilitiesSwitcher(coreSetup, plugins.licensing.license$, this.log);
setupSavedObjects(coreSetup.savedObjects);

const getMlSavedObjectsClient = (request: KibanaRequest): SavedObjectsClientContract | null => {
if (this.savedObjectsStart === null) {
return null;
}
return this.savedObjectsStart.getScopedClient(request, {
includedHiddenTypes: ['ml-job'],
});
};
const { getInternalSavedObjectsClient, getMlSavedObjectsClient } = savedObjectClientsFactory(
() => this.savedObjectsStart
);

const routeInit: RouteInitialization = {
router: coreSetup.http.createRouter(),
routeGuard: new RouteGuard(this.mlLicense, getMlSavedObjectsClient),
routeGuard: new RouteGuard(
this.mlLicense,
getMlSavedObjectsClient,
getInternalSavedObjectsClient,
plugins.spaces,
() => this.isMlReady
),
mlLicense: this.mlLicense,
};

Expand Down Expand Up @@ -176,7 +186,9 @@ export class MlServerPlugin implements Plugin<MlPluginSetup, MlPluginStart, Plug
plugins.spaces,
plugins.cloud,
resolveMlCapabilities,
() => this.clusterClient
() => this.clusterClient,
() => getInternalSavedObjectsClient(),
() => this.isMlReady
),
};
}
Expand All @@ -185,6 +197,16 @@ export class MlServerPlugin implements Plugin<MlPluginSetup, MlPluginStart, Plug
this.capabilities = coreStart.capabilities;
this.clusterClient = coreStart.elasticsearch.client;
this.savedObjectsStart = coreStart.savedObjects;

// check whether the job saved objects exist
// and create them if needed.
const { initializeJobs } = jobSavedObjectsInitializationFactory(
coreStart,
this.spacesPlugin !== undefined
);
initializeJobs().finally(() => {
Copy link
Member

Choose a reason for hiding this comment

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

To improve visibility into the status of this operation, we could hook into Core's status service (exposed on CoreSetup). This would allow the platform, ML consumers, and end users to understand why jobs may be missing in the event of a failure here.

Don't consider this a blocker for merging, but I'd recommend at least exploring this in a followup. If nothing else, it will help us triage failures with the inevitable SDH comes in

Copy link
Member Author

@jgowdyelastic jgowdyelastic Nov 6, 2020

Choose a reason for hiding this comment

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

"inevitable" excuse me?!

Copy link
Member

Choose a reason for hiding this comment

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

Ha, that wasn't at all a knock on your work here, sorry if it came across that way. Coordinating something like this with a distributed system is bound to have some growing pains. We continue to have them on our team with respect to the authorization system we've built, despite our best efforts.

Copy link
Member Author

@jgowdyelastic jgowdyelastic Nov 6, 2020

Choose a reason for hiding this comment

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

i know, sorry! i just thought that was funny.

this.setMlReady();
});
}

public stop() {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/ml/server/routes/saved_objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { wrapError } from '../client/error_wrapper';
import { RouteInitialization } from '../types';
import { checksFactory } from '../saved_objects';
import { checksFactory, repairFactory } from '../saved_objects';
import { jobsAndSpaces, repairJobObjects } from './schemas/saved_objects';

/**
Expand Down Expand Up @@ -67,7 +67,7 @@ export function savedObjectsRoutes({ router, routeGuard }: RouteInitialization)
routeGuard.fullLicenseAPIGuard(async ({ client, request, response, jobSavedObjectService }) => {
try {
const { simulate } = request.query;
const { repairJobs } = checksFactory(client, jobSavedObjectService);
const { repairJobs } = repairFactory(client, jobSavedObjectService);
const savedObjects = await repairJobs(simulate);

return response.ok({
Expand Down Expand Up @@ -100,7 +100,7 @@ export function savedObjectsRoutes({ router, routeGuard }: RouteInitialization)
routeGuard.fullLicenseAPIGuard(async ({ client, request, response, jobSavedObjectService }) => {
try {
const { simulate } = request.query;
const { initSavedObjects } = checksFactory(client, jobSavedObjectService);
const { initSavedObjects } = repairFactory(client, jobSavedObjectService);
const savedObjects = await initSavedObjects(simulate);

return response.ok({
Expand Down
8 changes: 2 additions & 6 deletions x-pack/plugins/ml/server/routes/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { schema } from '@kbn/config-schema';
import { Request } from '@hapi/hapi';
import { IScopedClusterClient } from 'kibana/server';
import { wrapError } from '../client/error_wrapper';
import { mlLog } from '../client/log';
import { mlLog } from '../lib/log';
import { capabilitiesProvider } from '../lib/capabilities';
import { spacesUtilsProvider } from '../lib/spaces_utils';
import { RouteInitialization, SystemRouteDeps } from '../types';
Expand Down Expand Up @@ -117,11 +117,7 @@ export function systemRoutes(
},
routeGuard.basicLicenseAPIGuard(async ({ mlClient, request, response }) => {
try {
// if spaces is disabled force isMlEnabledInSpace to be true
const { isMlEnabledInSpace } =
spaces !== undefined
? spacesUtilsProvider(spaces, (request as unknown) as Request)
: { isMlEnabledInSpace: async () => true };
const { isMlEnabledInSpace } = spacesUtilsProvider(spaces, (request as unknown) as Request);

const mlCapabilities = await resolveMlCapabilities(request);
if (mlCapabilities === null) {
Expand Down
Loading