Skip to content

Commit

Permalink
[ML] Fixing capabilities when ML is disabled in elasticsearch (#143622)
Browse files Browse the repository at this point in the history
Moves the initialisation of ML saved objects and the auto sync task to
after a license check has been performed. If ML is not enabled or the
license is not platinum or trial we do not initialise the saved objects
or create the auto sync task.

Updates the license checks to react to license changes. If the license
changes from full (platinum or trial) to something else (e.g. basic) we
disable the auto sync tasks.
If the license changes from non-full to full we initialise the saved
objects and start the task.

Removes the `canAccessMl` capability in favour of explicit capabilities
checks. `canAccessMl` was badly named and as a result was being misused
by a few plugins, thinking it was the correct capability to check to see
if ML is available, when really it was the very minimum check to cover
our basic licensed features.

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
jgowdyelastic and kibanamachine authored Dec 13, 2022
1 parent 47ad5ed commit 1d6bac7
Show file tree
Hide file tree
Showing 21 changed files with 107 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ const DEFAULT_VALUES = {
canUseMlInferencePipeline: true,
capabilities: {
ml: {
canAccessML: true,
canGetTrainedModels: true,
},
},
hasIndexIngestionPipeline: true,
hasPlatinumLicense: true,
ingestionMethod: 'crawler',
};

Expand All @@ -40,22 +39,14 @@ describe('add inference pipeline button', () => {
const button = wrapper.find(EuiButton);
expect(button.text()).toBe('Add Inference Pipeline');
});
it('renders permission tooltip with no ml access', () => {
it('renders permission tooltip when user cannot get trained models', () => {
setMockValues({ ...DEFAULT_VALUES, capabilities: {} });
const wrapper = mount(<AddMLInferencePipelineButton onClick={onClick} />);
expect(wrapper.find(EuiButton)).toHaveLength(1);
expect(wrapper.find(EuiToolTip)).toHaveLength(1);
const tooltip = wrapper.find(EuiToolTip);
expect(tooltip.prop('content')).toContain('permission');
});
it('renders permission tooltip with no platinum license', () => {
setMockValues({ ...DEFAULT_VALUES, hasPlatinumLicense: false });
const wrapper = mount(<AddMLInferencePipelineButton onClick={onClick} />);
expect(wrapper.find(EuiButton)).toHaveLength(1);
expect(wrapper.find(EuiToolTip)).toHaveLength(1);
const tooltip = wrapper.find(EuiToolTip);
expect(tooltip.prop('content')).toContain('permission');
});
it('renders copy & customize tooltip with index pipeline', () => {
setMockValues({ ...DEFAULT_VALUES, hasIndexIngestionPipeline: false });
const wrapper = mount(<AddMLInferencePipelineButton onClick={onClick} />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { EuiButton, EuiToolTip } from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { KibanaLogic } from '../../../../../shared/kibana/kibana_logic';
import { LicensingLogic } from '../../../../../shared/licensing';
import { IndexViewLogic } from '../../index_view_logic';
import { PipelinesLogic } from '../pipelines_logic';

Expand All @@ -26,10 +25,8 @@ export const AddMLInferencePipelineButton: React.FC<AddMLInferencePipelineButton
const { capabilities } = useValues(KibanaLogic);
const { ingestionMethod } = useValues(IndexViewLogic);
const { canUseMlInferencePipeline, hasIndexIngestionPipeline } = useValues(PipelinesLogic);
const { hasPlatinumLicense } = useValues(LicensingLogic);
const hasMLPermissions = capabilities?.ml?.canAccessML ?? false;

if (!hasMLPermissions || !hasPlatinumLicense) {
const hasMLPermissions = capabilities?.ml?.canGetTrainedModels ?? false;
if (!hasMLPermissions) {
return (
<EuiToolTip
content={i18n.translate(
Expand Down
11 changes: 3 additions & 8 deletions x-pack/plugins/ml/common/license/ml_license.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,8 @@ export class MlLicense {
private _isMinimumLicense: boolean = false;
private _isFullLicense: boolean = false;
private _isTrialLicense: boolean = false;
private _initialized: boolean = false;

public setup(
license$: Observable<ILicense>,
postInitFunctions?: Array<(lic: MlLicense) => void>
) {
public setup(license$: Observable<ILicense>, callback?: (lic: MlLicense) => void) {
this._licenseSubscription = license$.subscribe(async (license) => {
const { isEnabled: securityIsEnabled } = license.getFeature('security');

Expand All @@ -45,10 +41,9 @@ export class MlLicense {
this._isFullLicense = isFullLicense(this._license);
this._isTrialLicense = isTrialLicense(this._license);

if (this._initialized === false && postInitFunctions !== undefined) {
postInitFunctions.forEach((f) => f(this));
if (callback !== undefined) {
callback(this);
}
this._initialized = true;
});
}

Expand Down
12 changes: 7 additions & 5 deletions x-pack/plugins/ml/common/types/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@ import { ML_ALERT_TYPES } from '../constants/alerts';

export const apmUserMlCapabilities = {
canGetJobs: false,
canAccessML: false,
};

export const userMlCapabilities = {
canAccessML: false,
// Anomaly Detection
canGetJobs: false,
canGetDatafeeds: false,
Expand All @@ -39,6 +37,8 @@ export const userMlCapabilities = {
// Trained models
canGetTrainedModels: false,
canTestTrainedModels: false,
canGetFieldInfo: false,
canGetMlInfo: false,
};

export const adminMlCapabilities = {
Expand Down Expand Up @@ -83,9 +83,11 @@ export type AdminMlCapabilities = typeof adminMlCapabilities;
export type MlCapabilities = UserMlCapabilities & AdminMlCapabilities;
export type MlCapabilitiesKey = keyof MlCapabilities;

export const basicLicenseMlCapabilities = ['canAccessML', 'canFindFileStructure'] as Array<
keyof MlCapabilities
>;
export const basicLicenseMlCapabilities = [
'canFindFileStructure',
'canGetFieldInfo',
'canGetMlInfo',
] as Array<keyof MlCapabilities>;

export function getDefaultCapabilities(): MlCapabilities {
return {
Expand Down
13 changes: 6 additions & 7 deletions x-pack/plugins/ml/public/application/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,12 @@ export const renderApp = (

appMountParams.onAppLeave((actions) => actions.default());

const mlLicense = setLicenseCache(deps.licensing, coreStart.application, [
() =>
ReactDOM.render(
<App coreStart={coreStart} deps={deps} appMountParams={appMountParams} />,
appMountParams.element
),
]);
const mlLicense = setLicenseCache(deps.licensing, coreStart.application, () =>
ReactDOM.render(
<App coreStart={coreStart} deps={deps} appMountParams={appMountParams} />,
appMountParams.element
)
);

return () => {
mlLicense.unsubscribe();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ let mlLicense: MlClientLicense | null = null;
export function setLicenseCache(
licensingStart: LicensingPluginStart,
application: CoreStart['application'],
postInitFunctions?: Array<(lic: MlLicense) => void>
callback?: (lic: MlLicense) => void
) {
mlLicense = new MlClientLicense(application);
mlLicense.setup(licensingStart.license$, postInitFunctions);
mlLicense.setup(licensingStart.license$, callback);
return mlLicense;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,12 @@ describe('MlClientLicense', () => {

const license$ = new Subject();

mlLicense.setup(license$ as Observable<ILicense>, [
(license) => {
// when passed in via postInitFunction callback, the license should be valid
// even if the license$ observable gets triggered after this setup.
expect(license.isFullLicense()).toBe(true);
done();
},
]);
mlLicense.setup(license$ as Observable<ILicense>, (license) => {
// when passed in via postInitFunction callback, the license should be valid
// even if the license$ observable gets triggered after this setup.
expect(license.isFullLicense()).toBe(true);
done();
});

license$.next({
check: () => ({ state: 'valid' }),
Expand Down
7 changes: 3 additions & 4 deletions x-pack/plugins/ml/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,12 @@ export class MlPlugin implements Plugin<MlPluginSetup, MlPluginStart> {

const licensing = pluginsSetup.licensing.license$.pipe(take(1));
licensing.subscribe(async (license) => {
const mlEnabled = isMlEnabled(license);
const fullLicense = isFullLicense(license);
const [coreStart, pluginStart] = await core.getStartServices();
const { capabilities } = coreStart.application;

if (isMlEnabled(license)) {
if (mlEnabled) {
// add ML to home page
if (pluginsSetup.home) {
registerFeature(pluginsSetup.home);
Expand All @@ -179,9 +181,6 @@ export class MlPlugin implements Plugin<MlPluginSetup, MlPluginStart> {
registerCasesAttachments,
} = await import('./register_helper');

const mlEnabled = isMlEnabled(license);
const fullLicense = isFullLicense(license);

if (pluginsSetup.maps) {
// Pass capabilites.ml.canGetJobs as minimum permission to show anomalies card in maps layers
const canGetJobs = capabilities.ml?.canGetJobs === true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('check_capabilities', () => {
);
const { capabilities } = await getCapabilities();
const count = Object.keys(capabilities).length;
expect(count).toBe(37);
expect(count).toBe(38);
});
});

Expand All @@ -65,7 +65,8 @@ describe('check_capabilities', () => {
expect(mlFeatureEnabledInSpace).toBe(true);
expect(isPlatinumOrTrialLicense).toBe(true);

expect(capabilities.canAccessML).toBe(true);
expect(capabilities.canGetFieldInfo).toBe(true);
expect(capabilities.canGetMlInfo).toBe(true);
expect(capabilities.canGetJobs).toBe(true);
expect(capabilities.canGetDatafeeds).toBe(true);
expect(capabilities.canGetCalendars).toBe(true);
Expand Down Expand Up @@ -118,7 +119,8 @@ describe('check_capabilities', () => {
expect(mlFeatureEnabledInSpace).toBe(true);
expect(isPlatinumOrTrialLicense).toBe(true);

expect(capabilities.canAccessML).toBe(true);
expect(capabilities.canGetFieldInfo).toBe(true);
expect(capabilities.canGetMlInfo).toBe(true);
expect(capabilities.canGetJobs).toBe(true);
expect(capabilities.canGetDatafeeds).toBe(true);
expect(capabilities.canGetCalendars).toBe(true);
Expand Down Expand Up @@ -171,7 +173,8 @@ describe('check_capabilities', () => {
expect(mlFeatureEnabledInSpace).toBe(true);
expect(isPlatinumOrTrialLicense).toBe(true);

expect(capabilities.canAccessML).toBe(true);
expect(capabilities.canGetFieldInfo).toBe(true);
expect(capabilities.canGetMlInfo).toBe(true);
expect(capabilities.canGetJobs).toBe(true);
expect(capabilities.canGetDatafeeds).toBe(true);
expect(capabilities.canGetCalendars).toBe(true);
Expand Down Expand Up @@ -224,7 +227,8 @@ describe('check_capabilities', () => {
expect(mlFeatureEnabledInSpace).toBe(true);
expect(isPlatinumOrTrialLicense).toBe(true);

expect(capabilities.canAccessML).toBe(true);
expect(capabilities.canGetFieldInfo).toBe(true);
expect(capabilities.canGetMlInfo).toBe(true);
expect(capabilities.canGetJobs).toBe(true);
expect(capabilities.canGetDatafeeds).toBe(true);
expect(capabilities.canGetCalendars).toBe(true);
Expand Down Expand Up @@ -277,7 +281,8 @@ describe('check_capabilities', () => {
expect(mlFeatureEnabledInSpace).toBe(false);
expect(isPlatinumOrTrialLicense).toBe(true);

expect(capabilities.canAccessML).toBe(false);
expect(capabilities.canGetFieldInfo).toBe(false);
expect(capabilities.canGetMlInfo).toBe(false);
expect(capabilities.canGetJobs).toBe(false);
expect(capabilities.canGetDatafeeds).toBe(false);
expect(capabilities.canGetCalendars).toBe(false);
Expand Down Expand Up @@ -332,7 +337,8 @@ describe('check_capabilities', () => {
expect(mlFeatureEnabledInSpace).toBe(false);
expect(isPlatinumOrTrialLicense).toBe(false);

expect(capabilities.canAccessML).toBe(false);
expect(capabilities.canGetFieldInfo).toBe(false);
expect(capabilities.canGetMlInfo).toBe(false);
expect(capabilities.canGetJobs).toBe(false);
expect(capabilities.canGetDatafeeds).toBe(false);
expect(capabilities.canGetCalendars).toBe(false);
Expand Down
16 changes: 8 additions & 8 deletions x-pack/plugins/ml/server/lib/ml_client/ml_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ export function getMlClient(
if (error.statusCode === 404) {
throw new MLJobNotFound(error.body.error.reason);
}
throw error.body ?? error;
throw error;
}
},
async getDataFrameAnalyticsStats(...p: Parameters<MlClient['getDataFrameAnalyticsStats']>) {
Expand Down Expand Up @@ -317,7 +317,7 @@ export function getMlClient(
if (error.statusCode === 404) {
throw new MLJobNotFound(error.body.error.reason);
}
throw error.body ?? error;
throw error;
}
},
async getDatafeedStats(...p: Parameters<MlClient['getDatafeedStats']>) {
Expand All @@ -342,7 +342,7 @@ export function getMlClient(
if (error.statusCode === 404) {
throw new MLJobNotFound(error.body.error.reason);
}
throw error.body ?? error;
throw error;
}
},
async getDatafeeds(...p: Parameters<MlClient['getDatafeeds']>) {
Expand All @@ -367,7 +367,7 @@ export function getMlClient(
if (error.statusCode === 404) {
throw new MLJobNotFound(error.body.error.reason);
}
throw error.body ?? error;
throw error;
}
},
async getFilters(...p: Parameters<MlClient['getFilters']>) {
Expand Down Expand Up @@ -405,7 +405,7 @@ export function getMlClient(
if (error.statusCode === 404) {
throw new MLJobNotFound(error.body.error.reason);
}
throw error.body ?? error;
throw error;
}
},
async getJobs(...p: Parameters<MlClient['getJobs']>) {
Expand Down Expand Up @@ -437,7 +437,7 @@ export function getMlClient(
if (error.statusCode === 404) {
throw new MLJobNotFound(error.body.error.reason);
}
throw error.body ?? error;
throw error;
}
},
async getModelSnapshots(...p: Parameters<MlClient['getModelSnapshots']>) {
Expand Down Expand Up @@ -466,7 +466,7 @@ export function getMlClient(
if (error.statusCode === 404) {
throw new MLModelNotFound(error.body.error.reason);
}
throw error.body ?? error;
throw error;
}
},
async getTrainedModelsStats(...p: Parameters<MlClient['getTrainedModelsStats']>) {
Expand All @@ -483,7 +483,7 @@ export function getMlClient(
if (error.statusCode === 404) {
throw new MLModelNotFound(error.body.error.reason);
}
throw error.body ?? error;
throw error;
}
},
async startTrainedModelDeployment(...p: Parameters<MlClient['startTrainedModelDeployment']>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
*/

import { i18n } from '@kbn/i18n';
import type { HomeServerPluginSetup } from '@kbn/home-plugin/server';
import { MlLicense } from '../../../common/license';
import { PluginsSetup } from '../../types';

export function initSampleDataSets(mlLicense: MlLicense, plugins: PluginsSetup) {
export function initSampleDataSets(mlLicense: MlLicense, home: HomeServerPluginSetup) {
if (mlLicense.isMlEnabled() && mlLicense.isFullLicense()) {
const sampleDataLinkLabel = i18n.translate('xpack.ml.sampleDataLinkLabel', {
defaultMessage: 'ML jobs',
});
const { addAppLinksToSampleDataset } = plugins.home.sampleData;
const { addAppLinksToSampleDataset } = home.sampleData;
const getCreateJobPath = (jobId: string, dataViewId: string) =>
`/app/ml/modules/check_view_or_create?id=${jobId}&index=${dataViewId}`;

Expand Down
Loading

0 comments on commit 1d6bac7

Please sign in to comment.