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] Add ML saved objects to Kibana saved objects management page #205177

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -386,9 +386,30 @@ export class Table extends PureComponent<TableProps, TableState> {
const activeActionContents = this.state.activeAction?.render() ?? null;
const exceededResultCount = totalItemCount > MAX_PAGINATED_ITEM;

const hasMlObjects = selectedSavedObjects.some(({ type }) => type === 'ml-job');

const anySelected = selectedSavedObjects.length > 0;
const allHidden =
anySelected && selectedSavedObjects.every(({ meta: { hiddenType } }) => hiddenType);

const deleteTooltip = () => {
if (hasMlObjects) {
return (
<FormattedMessage
id="savedObjectsManagement.objectsTable.table.hasMlObjects.deleteDisabledTooltip"
defaultMessage="Machine learning objects can’t be deleted."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest editing this to say something like Machine learning objects can only be deleted in the Machine Learning management page (last part of that message needs some work!).

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated 1450c34 (#205177)
Screenshot 2025-01-02 at 15 59 19

@szabosteve Follow-up on my slack message, would love to get some feedback for this one liner.

Copy link
Contributor

Choose a reason for hiding this comment

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

@qn895 @peteharverson This tooltip message works fine. We could maybe phrase it a bit more action-oriented, something along the lines of:

Navigate to the Machine Learning management page to delete machine learning objects.

Copy link
Contributor

Choose a reason for hiding this comment

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

That text sounds good to me. Thanks @szabosteve

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed, until the new management pages are in, let's change this to say Navigate to the Machine Learning pages to delete machine learning objects.

/>
);
}
if (allHidden) {
return (
<FormattedMessage
id="savedObjectsManagement.objectsTable.table.deleteDisabledTooltip"
defaultMessage="Selected objects can’t be deleted because they are hidden objects."
/>
);
}
};
return (
<Fragment>
{activeActionContents}
Expand All @@ -406,22 +427,18 @@ export class Table extends PureComponent<TableProps, TableState> {
<EuiToolTip
data-test-subj="deleteSOToolTip"
key="deleteSOToolTip"
content={
allHidden ? (
<FormattedMessage
id="savedObjectsManagement.objectsTable.table.deleteDisabledTooltip"
defaultMessage="Selected objects can’t be deleted because they are hidden objects."
/>
) : undefined
}
content={deleteTooltip()}
>
<EuiButton
key="deleteSO"
iconType="trash"
color="danger"
onClick={onDelete}
isDisabled={
!anySelected || allHidden || !capabilities.savedObjectsManagement.delete
hasMlObjects ||
!anySelected ||
allHidden ||
!capabilities.savedObjectsManagement.delete
}
title={
capabilities.savedObjectsManagement.delete
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,13 @@ import {
ExportModal,
} from './components';

// Saved objects for ML job are not importable/exportable because they are wrappers around ES objects
const DISABLED_TYPES_FOR_EXPORT = new Set(['ml-job']);

interface ExportAllOption {
id: string;
label: string;
disabled?: boolean;
}

export interface SavedObjectsTableProps {
Expand Down Expand Up @@ -186,18 +190,18 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb
([id, count]) => ({
id,
label: `${id} (${count || 0})`,
disabled: DISABLED_TYPES_FOR_EXPORT.has(id),
})
);
const exportAllSelectedOptions: Record<string, boolean> = exportAllOptions.reduce(
(record, { id }) => {
return {
...record,
[id]: true,
[id]: DISABLED_TYPES_FOR_EXPORT.has(id) ? false : true,
};
},
{}
);

// Fetch all the saved objects that exist so we can accurately populate the counts within
// the table filter dropdown.
const savedObjectCounts = await getSavedObjectCounts({
Expand Down Expand Up @@ -420,7 +424,7 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb
const { notifications, http, taggingApi, allowedTypes } = this.props;
const { queryText, selectedTags } = parseQuery(activeQuery, allowedTypes);
const exportTypes = Object.entries(exportAllSelectedOptions).reduce((accum, [id, selected]) => {
if (selected) {
if (selected && !DISABLED_TYPES_FOR_EXPORT.has(id)) {
accum.push(id);
}
return accum;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@

import type { ErrorType } from '@kbn/ml-error-utils';

export type JobType = 'anomaly-detector' | 'data-frame-analytics';
export const AD_SAVED_OBJECT_TYPE = 'anomaly-detector';
export const DFA_SAVED_OBJECT_TYPE = 'data-frame-analytics';

export type JobType = typeof AD_SAVED_OBJECT_TYPE | typeof DFA_SAVED_OBJECT_TYPE;
export type TrainedModelType = 'trained-model';
export type MlSavedObjectType = JobType | TrainedModelType;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { mlJob, mlTrainedModel, mlModule } from './mappings';

import { migrations } from './migrations';
import {
AD_SAVED_OBJECT_TYPE,
DFA_SAVED_OBJECT_TYPE,
ML_JOB_SAVED_OBJECT_TYPE,
ML_MODULE_SAVED_OBJECT_TYPE,
ML_TRAINED_MODEL_SAVED_OBJECT_TYPE,
Expand All @@ -27,13 +29,38 @@ interface MlModuleAttributes {
datafeeds: object[];
}

const AD_PREFIX = `${AD_SAVED_OBJECT_TYPE}-`;
const DFA_PREFIX = `${DFA_SAVED_OBJECT_TYPE}-`;

export function setupSavedObjects(savedObjects: SavedObjectsServiceSetup) {
savedObjects.registerType({
name: ML_JOB_SAVED_OBJECT_TYPE,
hidden: false,
namespaceType: 'multiple',
migrations,
mappings: mlJob,
management: {
importableAndExportable: true,
peteharverson marked this conversation as resolved.
Show resolved Hide resolved
getTitle(obj) {
return obj.id;
},
getInAppUrl(obj) {
if (obj.id.startsWith(AD_PREFIX)) {
const jobId = obj.id.slice(AD_PREFIX.length);
return {
path: `/app/management/insightsAndAlerting/jobsListLink/${jobId}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Passing the jobId in the link is wasted currently as we don't use that in the ML management page to restore a filter to the query bar. But I guess it makes more sense to wait till the new management page is ready and then edit this link to provide a filtered link to that page?

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed, I'm removing the links for all ML objects for now until we have the PR for management page ready

uiCapabilitiesPath: 'ml.canGetJobs',
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to add additional capabilities checks to control what actions a user can perform and whether or not the saved objects are listed?

I think should not list the SOs if the user does not have the minimum read permissions and should only be able to edit them if they have write permissions, e.g. for AD SOs:
canGetJobs AD job saved objects are listed but are read only.
canGetJobs && canCreateJobs AD job saved objects are listed and can be edited

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Added capabilities check here and here

};
}
if (obj.id.startsWith(DFA_PREFIX)) {
// @TODO: Temporarily hide link until new management link is ready
return undefined;
}
return undefined;
},
icon: 'machineLearningApp',
displayName: 'ml',
peteharverson marked this conversation as resolved.
Show resolved Hide resolved
},
});
savedObjects.registerType({
name: ML_TRAINED_MODEL_SAVED_OBJECT_TYPE,
Expand Down