From 9a729291419f2bbc1cbb24eb70da2b7f00c51b03 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Mon, 1 Jun 2020 23:55:34 -0500 Subject: [PATCH 01/17] [Transform] Add ability to delete dest index & index pattern when deleting job --- x-pack/plugins/transform/common/index.ts | 17 ++ .../transform/public/app/hooks/index.ts | 2 +- .../transform/public/app/hooks/use_api.ts | 15 +- .../public/app/hooks/use_delete_transform.tsx | 212 +++++++++++++++- .../transform_list/action_delete.tsx | 127 ++++++++-- .../server/routes/api/error_utils.ts | 10 +- .../transform/server/routes/api/transforms.ts | 228 +++++++++++++++++- 7 files changed, 570 insertions(+), 41 deletions(-) diff --git a/x-pack/plugins/transform/common/index.ts b/x-pack/plugins/transform/common/index.ts index d7a791e78b3ab..3fb2c2071c483 100644 --- a/x-pack/plugins/transform/common/index.ts +++ b/x-pack/plugins/transform/common/index.ts @@ -38,3 +38,20 @@ export interface ResultData { export interface TransformEndpointResult { [key: string]: ResultData; } + +export interface DeleteTransformEndpoint { + transformsInfo: TransformEndpointRequest[]; + deleteDestIndex?: boolean; + deleteDestIndexPattern?: boolean; +} + +export interface DeleteTransformStatus { + transformJobDeleted: ResultData; + destIndexDeleted?: ResultData; + destIndexPatternDeleted?: ResultData; + destinationIndex?: string | undefined; +} + +export interface DeleteTransformEndpointResult { + [key: string]: DeleteTransformStatus; +} diff --git a/x-pack/plugins/transform/public/app/hooks/index.ts b/x-pack/plugins/transform/public/app/hooks/index.ts index a36550bcd8e57..b439afe2b2165 100644 --- a/x-pack/plugins/transform/public/app/hooks/index.ts +++ b/x-pack/plugins/transform/public/app/hooks/index.ts @@ -6,7 +6,7 @@ export { useApi } from './use_api'; export { useGetTransforms } from './use_get_transforms'; -export { useDeleteTransforms } from './use_delete_transform'; +export { useDeleteTransforms, useDeleteIndexAndTargetIndex } from './use_delete_transform'; export { useStartTransforms } from './use_start_transform'; export { useStopTransforms } from './use_stop_transform'; export { useRequest } from './use_request'; diff --git a/x-pack/plugins/transform/public/app/hooks/use_api.ts b/x-pack/plugins/transform/public/app/hooks/use_api.ts index 466575a99b2b4..d3b08dbd11be6 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_api.ts +++ b/x-pack/plugins/transform/public/app/hooks/use_api.ts @@ -4,7 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import { TransformId, TransformEndpointRequest, TransformEndpointResult } from '../../../common'; +import { + TransformId, + TransformEndpointRequest, + TransformEndpointResult, + DeleteTransformEndpointResult, +} from '../../../common'; import { API_BASE_PATH } from '../../../common/constants'; import { useAppDependencies } from '../app_dependencies'; @@ -37,9 +42,13 @@ export const useApi = () => { body: JSON.stringify(transformConfig), }); }, - deleteTransforms(transformsInfo: TransformEndpointRequest[]): Promise { + deleteTransforms( + transformsInfo: TransformEndpointRequest[], + deleteDestIndex: boolean | undefined, + deleteDestIndexPattern: boolean | undefined + ): Promise { return http.post(`${API_BASE_PATH}delete_transforms`, { - body: JSON.stringify(transformsInfo), + body: JSON.stringify({ transformsInfo, deleteDestIndex, deleteDestIndexPattern }), }); }, getTransformsPreview(obj: PreviewRequestBody): Promise { diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index 0215d723188b1..01200753a7140 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -4,12 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import { i18n } from '@kbn/i18n'; +import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; +import { CoreSetup } from 'kibana/public'; import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public'; -import { TransformEndpointRequest, TransformEndpointResult } from '../../../common'; +import { TransformEndpointRequest, DeleteTransformEndpointResult } from '../../../common'; import { getErrorMessage } from '../../shared_imports'; @@ -18,38 +20,230 @@ import { TransformListRow, refreshTransformList$, REFRESH_TRANSFORM_LIST_STATE } import { ToastNotificationText } from '../components'; import { useApi } from './use_api'; +import { API_BASE_PATH } from '../../../common/constants'; +import { IIndexPattern } from '../../../../../../src/plugins/data/common/index_patterns'; + +export const extractErrorMessage = ( + error: CustomHttpResponseOptions | undefined | string +): string | undefined => { + if (typeof error === 'string') { + return error; + } + + if (error?.body) { + if (typeof error.body === 'string') { + return error.body; + } + if (typeof error.body === 'object' && 'message' in error.body) { + if (typeof error.body.message === 'string') { + return error.body.message; + } + // @ts-ignore + if (typeof (error.body.message?.msg === 'string')) { + // @ts-ignore + return error.body.message?.msg; + } + } + } + return undefined; +}; + +export const canDeleteIndex = async (http: CoreSetup['http']) => { + const privilege = await http.get(`${API_BASE_PATH}privileges`); + if (!privilege) { + return false; + } + return privilege.hasAllPrivileges; +}; + +export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { + const { http, savedObjects } = useAppDependencies(); + const toastNotifications = useToastNotifications(); + + const [deleteDestIndex, setDeleteDestIndex] = useState(true); + const [deleteIndexPattern, setDeleteIndexPattern] = useState(true); + const [userCanDeleteIndex, setUserCanDeleteIndex] = useState(false); + const [indexPatternExists, setIndexPatternExists] = useState(false); + const toggleDeleteIndex = useCallback(() => setDeleteDestIndex(!deleteDestIndex), [ + deleteDestIndex, + ]); + const toggleDeleteIndexPattern = useCallback(() => setDeleteIndexPattern(!deleteIndexPattern), [ + deleteIndexPattern, + ]); + const savedObjectsClient = savedObjects.client; + + const checkIndexPatternExists = useCallback( + async (indexName: string) => { + try { + const response = await savedObjectsClient.find({ + type: 'index-pattern', + perPage: 10, + search: `"${indexName}"`, + searchFields: ['title'], + fields: ['title'], + }); + const ip = response.savedObjects.find( + (obj) => obj.attributes.title.toLowerCase() === indexName.toLowerCase() + ); + if (ip !== undefined) { + setIndexPatternExists(true); + } + } catch (e) { + const error = extractErrorMessage(e); + + toastNotifications.addDanger( + i18n.translate( + 'xpack.ml.dataframe.analyticsList.errorWithCheckingIfIndexPatternExistsNotificationErrorMessage', + { + defaultMessage: + 'An error occurred checking if index pattern {indexPattern} exists: {error}', + values: { indexPattern: 'blah', error }, + } + ) + ); + } + }, + [savedObjectsClient, toastNotifications] + ); + + const checkUserIndexPermission = useCallback(() => { + try { + const userCanDelete = canDeleteIndex(http); + if (userCanDelete) { + setUserCanDeleteIndex(true); + } + } catch (e) { + toastNotifications.addDanger( + i18n.translate( + 'xpack.transform.transformList.errorWithCheckingIfUserCanDeleteIndexNotificationErrorMessage', + { + defaultMessage: 'An error occurred checking if user can delete destination index', + } + ) + ); + } + }, [http, toastNotifications]); + + useEffect(() => { + checkUserIndexPermission(); + + if (items.length === 1) { + const config = items[0].config; + const destinationIndex = Array.isArray(config.dest.index) + ? config.dest.index[0] + : config.dest.index; + checkIndexPatternExists(destinationIndex); + } + }, [checkIndexPatternExists, checkUserIndexPermission, items]); + + return { + userCanDeleteIndex, + deleteDestIndex, + indexPatternExists, + deleteIndexPattern, + toggleDeleteIndex, + toggleDeleteIndexPattern, + }; +}; export const useDeleteTransforms = () => { const { overlays } = useAppDependencies(); const toastNotifications = useToastNotifications(); const api = useApi(); - return async (transforms: TransformListRow[]) => { + return async ( + transforms: TransformListRow[], + shouldDeleteDestIndex: boolean, + shouldDeleteDestIndexPattern: boolean + ) => { const transformsInfo: TransformEndpointRequest[] = transforms.map((tf) => ({ id: tf.config.id, state: tf.stats.state, })); try { - const results: TransformEndpointResult = await api.deleteTransforms(transformsInfo); + const results: DeleteTransformEndpointResult = await api.deleteTransforms( + transformsInfo, + shouldDeleteDestIndex, + shouldDeleteDestIndexPattern + ); for (const transformId in results) { // hasOwnProperty check to ensure only properties on object itself, and not its prototypes if (results.hasOwnProperty(transformId)) { - if (results[transformId].success === true) { + const status = results[transformId]; + const destinationIndex = status.destinationIndex; + + if (status.transformJobDeleted?.success) { toastNotifications.addSuccess( i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { - defaultMessage: 'Request to delete transform {transformId} acknowledged.', + defaultMessage: + 'Request to delete data frame analytics job {transformId} acknowledged.', values: { transformId }, }) ); - } else { + } + if (status.transformJobDeleted?.error) { + const error = extractErrorMessage(status.transformJobDeleted.error); toastNotifications.addDanger( i18n.translate('xpack.transform.transformList.deleteTransformErrorMessage', { - defaultMessage: 'An error occurred deleting the transform {transformId}', - values: { transformId }, + defaultMessage: + 'An error occurred deleting the data frame analytics job {transformId}: {error}', + values: { transformId, error }, }) ); } + + if (status.destIndexDeleted?.success) { + toastNotifications.addSuccess( + i18n.translate( + 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexSuccessMessage', + { + defaultMessage: + 'Request to delete destination index {destinationIndex} acknowledged.', + values: { destinationIndex }, + } + ) + ); + } + if (status.destIndexDeleted?.error) { + const error = extractErrorMessage(status.destIndexDeleted.error); + toastNotifications.addDanger( + i18n.translate( + 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexErrorMessage', + { + defaultMessage: + 'An error occurred deleting destination index {destinationIndex}: {error}', + values: { destinationIndex, error }, + } + ) + ); + } + + if (status.destIndexPatternDeleted?.success) { + toastNotifications.addSuccess( + i18n.translate( + 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternSuccessMessage', + { + defaultMessage: + 'Request to delete index pattern {destinationIndex} acknowledged.', + values: { destinationIndex }, + } + ) + ); + } + if (status.destIndexPatternDeleted?.error) { + const error = extractErrorMessage(status.destIndexPatternDeleted.error); + toastNotifications.addDanger( + i18n.translate( + 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternErrorMessage', + { + defaultMessage: + 'An error occurred deleting index pattern {destinationIndex}: {error}', + values: { destinationIndex, error }, + } + ) + ); + } } } diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx index c20feba29f582..3a4ad38b79c21 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx @@ -12,11 +12,13 @@ import { EuiOverlayMask, EuiToolTip, EUI_MODAL_CONFIRM_BUTTON, + EuiFlexGroup, + EuiFlexItem, + EuiSwitch, } from '@elastic/eui'; - +import { FormattedMessage } from '@kbn/i18n/react'; import { TRANSFORM_STATE } from '../../../../../../common'; - -import { useDeleteTransforms } from '../../../../hooks'; +import { useDeleteTransforms, useDeleteIndexAndTargetIndex } from '../../../../hooks'; import { createCapabilityFailureMessage, AuthorizationContext, @@ -35,13 +37,25 @@ export const DeleteAction: FC = ({ items, forceDisable }) => const { canDeleteTransform } = useContext(AuthorizationContext).capabilities; const deleteTransforms = useDeleteTransforms(); + const { + userCanDeleteIndex, + deleteDestIndex, + indexPatternExists, + deleteIndexPattern, + toggleDeleteIndex, + toggleDeleteIndexPattern, + } = useDeleteIndexAndTargetIndex(items); const [isModalVisible, setModalVisible] = useState(false); const closeModal = () => setModalVisible(false); const deleteAndCloseModal = () => { setModalVisible(false); - deleteTransforms(items); + + const shouldDeleteDestIndex = userCanDeleteIndex && deleteDestIndex; + const shouldDeleteDestIndexPattern = + userCanDeleteIndex && indexPatternExists && deleteIndexPattern; + deleteTransforms(items, shouldDeleteDestIndex, shouldDeleteDestIndexPattern); }; const openModal = () => setModalVisible(true); @@ -71,17 +85,96 @@ export const DeleteAction: FC = ({ items, forceDisable }) => defaultMessage: 'Delete {transformId}', values: { transformId: items[0] && items[0].config.id }, }); - const bulkDeleteModalMessage = i18n.translate( - 'xpack.transform.transformList.bulkDeleteModalBody', - { - defaultMessage: - "Are you sure you want to delete {count, plural, one {this} other {these}} {count} {count, plural, one {transform} other {transforms}}? The transform's destination index and optional Kibana index pattern will not be deleted.", - values: { count: items.length }, - } + const bulkDeleteModalMessage = ( + <> +

+ +

+ + + { + + } + + + { + + } + + + + ); + + const deleteModalMessage = ( + +

+ +

+ + + {userCanDeleteIndex && ( + + )} + + + {userCanDeleteIndex && indexPatternExists && ( + + )} + + + ); - const deleteModalMessage = i18n.translate('xpack.transform.transformList.deleteModalBody', { - defaultMessage: `Are you sure you want to delete this transform? The transform's destination index and optional Kibana index pattern will not be deleted.`, - }); let deleteButton = ( = ({ items, forceDisable }) => if (disabled || !canDeleteTransform) { let content; if (disabled) { - content = isBulkAction === true ? bulkDeleteButtonDisabledText : deleteButtonDisabledText; + content = isBulkAction ? bulkDeleteButtonDisabledText : deleteButtonDisabledText; } else { content = createCapabilityFailureMessage('canDeleteTransform'); } @@ -117,7 +210,7 @@ export const DeleteAction: FC = ({ items, forceDisable }) => {isModalVisible && ( = ({ items, forceDisable }) => defaultFocusedButton={EUI_MODAL_CONFIRM_BUTTON} buttonColor="danger" > -

{isBulkAction === true ? bulkDeleteModalMessage : deleteModalMessage}

+ {isBulkAction ? bulkDeleteModalMessage : deleteModalMessage} )} diff --git a/x-pack/plugins/transform/server/routes/api/error_utils.ts b/x-pack/plugins/transform/server/routes/api/error_utils.ts index 295375794c04e..5a479e4f429f6 100644 --- a/x-pack/plugins/transform/server/routes/api/error_utils.ts +++ b/x-pack/plugins/transform/server/routes/api/error_utils.ts @@ -10,7 +10,11 @@ import { i18n } from '@kbn/i18n'; import { ResponseError, CustomHttpResponseOptions } from 'src/core/server'; -import { TransformEndpointRequest, TransformEndpointResult } from '../../../common'; +import { + TransformEndpointRequest, + TransformEndpointResult, + DeleteTransformEndpointResult, +} from '../../../common'; const REQUEST_TIMEOUT = 'RequestTimeout'; @@ -19,7 +23,7 @@ export function isRequestTimeout(error: any) { } interface Params { - results: TransformEndpointResult; + results: TransformEndpointResult | DeleteTransformEndpointResult; id: string; items: TransformEndpointRequest[]; action: string; @@ -59,7 +63,7 @@ export function fillResultsWithTimeouts({ results, id, items, action }: Params) }, }; - const newResults: TransformEndpointResult = {}; + const newResults: TransformEndpointResult | DeleteTransformEndpointResult = {}; return items.reduce((accumResults, currentVal) => { if (results[currentVal.id] === undefined) { diff --git a/x-pack/plugins/transform/server/routes/api/transforms.ts b/x-pack/plugins/transform/server/routes/api/transforms.ts index 55b2469a7f3a7..9dc3cac2306cc 100644 --- a/x-pack/plugins/transform/server/routes/api/transforms.ts +++ b/x-pack/plugins/transform/server/routes/api/transforms.ts @@ -5,7 +5,12 @@ */ import { schema } from '@kbn/config-schema'; -import { RequestHandler } from 'kibana/server'; +import { + KibanaResponseFactory, + RequestHandler, + RequestHandlerContext, + SavedObjectsClientContract, +} from 'kibana/server'; import { CallCluster } from 'src/legacy/core_plugins/elasticsearch'; import { wrapEsError } from '../../../../../legacy/server/lib/create_router/error_wrappers'; @@ -14,6 +19,10 @@ import { TransformEndpointResult, TransformId, TRANSFORM_STATE, + DeleteTransformEndpoint, + DeleteTransformStatus, + DeleteTransformEndpointResult, + ResultData, } from '../../../common'; import { RouteDependencies } from '../../types'; @@ -23,6 +32,7 @@ import { addBasePath } from '../index'; import { isRequestTimeout, fillResultsWithTimeouts, wrapError } from './error_utils'; import { schemaTransformId, SchemaTransformId } from './schema'; import { registerTransformsAuditMessagesRoutes } from './transforms_audit_messages'; +import { IIndexPattern } from '../../../../../../src/plugins/data/common/index_patterns'; enum TRANSFORM_ACTIONS { STOP = 'stop', @@ -36,6 +46,31 @@ interface StopOptions { waitForCompletion?: boolean; } +export async function canDeleteIndex( + indexName: string, + callAsCurrentUser: CallCluster, + license: RouteDependencies['license'] +): Promise { + if (!license.getStatus().isSecurityEnabled) { + // If security isn't enabled, let the user use app. + return true; + } + + const { has_all_requested: hasAllPrivileges } = await callAsCurrentUser('transport.request', { + path: '/_security/user/_has_privileges', + method: 'POST', + body: { + index: [ + { + names: [indexName], + privileges: ['delete_index'], + }, + ], + }, + }); + return hasAllPrivileges; +} + export function registerTransformsRoutes(routeDependencies: RouteDependencies) { const { router, license } = routeDependencies; router.get( @@ -174,15 +209,42 @@ export function registerTransformsRoutes(routeDependencies: RouteDependencies) { path: addBasePath('delete_transforms'), validate: { body: schema.maybe(schema.any()), + query: schema.object({ + /** + * Analytics Destination Index + */ + deleteDestIndex: schema.maybe(schema.boolean()), + deleteDestIndexPattern: schema.maybe(schema.boolean()), + }), }, }, license.guardApiRoute(async (ctx, req, res) => { - const transformsInfo = req.body as TransformEndpointRequest[]; + const { + transformsInfo, + deleteDestIndex, + deleteDestIndexPattern, + } = req.body as DeleteTransformEndpoint; try { - return res.ok({ - body: await deleteTransforms(transformsInfo, ctx.transform!.dataClient.callAsCurrentUser), - }); + if (deleteDestIndex || deleteDestIndexPattern) { + return res.ok({ + body: await deleteTransformsWithDestIndex( + transformsInfo, + deleteDestIndex, + deleteDestIndexPattern, + ctx, + license, + res + ), + }); + } else { + return res.ok({ + body: await deleteTransforms( + transformsInfo, + ctx.transform!.dataClient.callAsCurrentUser + ), + }); + } } catch (e) { return res.customError(wrapError(wrapEsError(e))); } @@ -242,7 +304,7 @@ async function deleteTransforms( transformsInfo: TransformEndpointRequest[], callAsCurrentUser: CallCluster ) { - const results: TransformEndpointResult = {}; + const results: DeleteTransformEndpointResult = {}; for (const transformInfo of transformsInfo) { const transformId = transformInfo.id; @@ -267,7 +329,7 @@ async function deleteTransforms( } await callAsCurrentUser('transform.deleteTransform', { transformId }); - results[transformId] = { success: true }; + results[transformId] = { transformJobDeleted: { success: true } }; } catch (e) { if (isRequestTimeout(e)) { return fillResultsWithTimeouts({ @@ -277,7 +339,157 @@ async function deleteTransforms( action: TRANSFORM_ACTIONS.DELETE, }); } - results[transformId] = { success: false, error: JSON.stringify(e) }; + results[transformId] = { transformJobDeleted: { success: false, error: JSON.stringify(e) } }; + } + } + return results; +} + +async function getIndexPatternId( + indexName: string, + savedObjectsClient: SavedObjectsClientContract +) { + const response = await savedObjectsClient.find({ + type: 'index-pattern', + perPage: 10, + search: `"${indexName}"`, + searchFields: ['title'], + fields: ['title'], + }); + + const ip = response.saved_objects.find( + (obj) => obj.attributes.title.toLowerCase() === indexName.toLowerCase() + ); + + return ip?.id; +} + +async function deleteDestIndexPatternById( + indexPatternId: string, + savedObjectsClient: SavedObjectsClientContract +) { + return await savedObjectsClient.delete('index-pattern', indexPatternId); +} + +async function deleteTransformsWithDestIndex( + transformsInfo: TransformEndpointRequest[], + deleteDestIndex: boolean | undefined, + deleteDestIndexPattern: boolean | undefined, + ctx: RequestHandlerContext, + license: RouteDependencies['license'], + response: KibanaResponseFactory +) { + const tempResults: TransformEndpointResult = {}; + const results: Record = {}; + + for (const transformInfo of transformsInfo) { + let destinationIndex: string | undefined; + const transformJobDeleted: ResultData = { success: false }; + const destIndexDeleted: ResultData = { success: false }; + const destIndexPatternDeleted: ResultData = { + success: false, + }; + const transformId = transformInfo.id; + try { + if (transformInfo.state === TRANSFORM_STATE.FAILED) { + try { + await ctx.transform!.dataClient.callAsCurrentUser('transform.stopTransform', { + transformId, + force: true, + waitForCompletion: true, + } as StopOptions); + } catch (e) { + if (isRequestTimeout(e)) { + return fillResultsWithTimeouts({ + results: tempResults, + id: transformId, + items: transformsInfo, + action: TRANSFORM_ACTIONS.DELETE, + }); + } + } + } + // Grab destination index info to delete + try { + const transformConfigs = await getTransforms( + { transformId }, + ctx.transform!.dataClient.callAsCurrentUser + ); + const transformConfig = transformConfigs.transforms[0]; + destinationIndex = Array.isArray(transformConfig.dest.index) + ? transformConfig.dest.index[0] + : transformConfig.dest.index; + } catch (getTransformConfigError) { + return response.customError(wrapError(getTransformConfigError)); + } + + // If user checks box to delete the destinationIndex associated with the job + if (destinationIndex && deleteDestIndex) { + // Verify if user has privilege to delete the destination index + const userCanDeleteDestIndex = await canDeleteIndex( + destinationIndex, + ctx.transform!.dataClient.callAsCurrentUser, + license + ); + // If user does have privilege to delete the index, then delete the index + if (userCanDeleteDestIndex) { + try { + await ctx.transform!.dataClient.callAsCurrentUser('indices.delete', { + index: destinationIndex, + }); + destIndexDeleted.success = true; + } catch (deleteIndexError) { + destIndexDeleted.error = wrapError(deleteIndexError); + } + } else { + return response.forbidden(); + } + } + + // Delete the index pattern if there's an index pattern that matches the name of dest index + if (destinationIndex && deleteDestIndexPattern) { + try { + const indexPatternId = await getIndexPatternId( + destinationIndex, + ctx.core.savedObjects.client + ); + if (indexPatternId) { + await deleteDestIndexPatternById(indexPatternId, ctx.core.savedObjects.client); + } + destIndexPatternDeleted.success = true; + } catch (deleteDestIndexPatternError) { + destIndexPatternDeleted.error = wrapError(deleteDestIndexPatternError); + } + } + + try { + await ctx.transform!.dataClient.callAsCurrentUser('transform.deleteTransform', { + transformId, + }); + transformJobDeleted.success = true; + } catch (deleteTransformJobError) { + transformJobDeleted.error = wrapError(deleteTransformJobError); + if (transformJobDeleted.error.statusCode === 404) { + return response.notFound(); + } + } + + results[transformId] = { + transformJobDeleted, + destIndexDeleted, + destIndexPatternDeleted, + destinationIndex, + }; + } catch (e) { + if (isRequestTimeout(e)) { + return fillResultsWithTimeouts({ + results, + id: transformInfo.id, + items: transformsInfo, + action: TRANSFORM_ACTIONS.DELETE, + }); + } + results[transformId] = { transformJobDeleted: { success: false, error: JSON.stringify(e) } }; } } return results; From a9620c6064993592731f1c824e442c222db681e2 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 2 Jun 2020 15:11:10 -0500 Subject: [PATCH 02/17] [Transform] Combine success messages when deleting job transforms --- .../components/toast_notification_text.tsx | 12 +- .../public/app/hooks/use_delete_transform.tsx | 162 ++++++++++++------ .../transform/server/routes/api/transforms.ts | 4 +- 3 files changed, 120 insertions(+), 58 deletions(-) diff --git a/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx b/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx index 1044081670523..dcbbca2746d99 100644 --- a/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx +++ b/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx @@ -29,9 +29,14 @@ const MAX_SIMPLE_MESSAGE_LENGTH = 140; interface ToastNotificationTextProps { overlays: CoreStart['overlays']; text: any; + previewTextLength?: number; } -export const ToastNotificationText: FC = ({ overlays, text }) => { +export const ToastNotificationText: FC = ({ + overlays, + text, + previewTextLength, +}) => { if (typeof text === 'string' && text.length <= MAX_SIMPLE_MESSAGE_LENGTH) { return text; } @@ -46,8 +51,9 @@ export const ToastNotificationText: FC = ({ overlays const unformattedText = text.message ? text.message : text; const formattedText = typeof unformattedText === 'object' ? JSON.stringify(text, null, 2) : text; - const previewText = `${formattedText.substring(0, 140)}${ - formattedText.length > 140 ? ' ...' : '' + const textLength = previewTextLength || 140; + const previewText = `${formattedText.substring(0, textLength)}${ + formattedText.length > textLength ? ' ...' : '' }`; const openModal = () => { diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index 01200753a7140..e6136e5d71873 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -44,6 +44,13 @@ export const extractErrorMessage = ( return error.body.message?.msg; } } + if (typeof error.body === 'object' && 'msg' in error.body) { + // @ts-ignore + if (typeof error.body.msg === 'string') { + // @ts-ignore + return error.body.msg; + } + } } return undefined; }; @@ -97,7 +104,7 @@ export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { { defaultMessage: 'An error occurred checking if index pattern {indexPattern} exists: {error}', - values: { indexPattern: 'blah', error }, + values: { indexPattern: indexName, error }, } ) ); @@ -133,6 +140,8 @@ export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { ? config.dest.index[0] : config.dest.index; checkIndexPatternExists(destinationIndex); + } else { + setIndexPatternExists(true); } }, [checkIndexPatternExists, checkUserIndexPermission, items]); @@ -167,86 +176,133 @@ export const useDeleteTransforms = () => { shouldDeleteDestIndex, shouldDeleteDestIndexPattern ); + const isBulk = Object.keys(results).length > 1; + const successCount: Record = { + transformJobDeleted: 0, + destIndexDeleted: 0, + destIndexPatternDeleted: 0, + }; for (const transformId in results) { // hasOwnProperty check to ensure only properties on object itself, and not its prototypes if (results.hasOwnProperty(transformId)) { const status = results[transformId]; const destinationIndex = status.destinationIndex; - if (status.transformJobDeleted?.success) { - toastNotifications.addSuccess( - i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { - defaultMessage: - 'Request to delete data frame analytics job {transformId} acknowledged.', - values: { transformId }, - }) - ); + // if we are only deleting one modal, show the success toast messages + if (!isBulk) { + if (status.transformJobDeleted?.success) { + toastNotifications.addSuccess( + i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { + defaultMessage: 'Request to delete transform job {transformId} acknowledged.', + values: { transformId }, + }) + ); + } + if (status.destIndexDeleted?.success) { + toastNotifications.addSuccess( + i18n.translate( + 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexSuccessMessage', + { + defaultMessage: + 'Request to delete destination index {destinationIndex} acknowledged.', + values: { destinationIndex }, + } + ) + ); + } + if (status.destIndexPatternDeleted?.success) { + toastNotifications.addSuccess( + i18n.translate( + 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternSuccessMessage', + { + defaultMessage: + 'Request to delete index pattern {destinationIndex} acknowledged.', + values: { destinationIndex }, + } + ) + ); + } + } else { + Object.keys(successCount).forEach((key) => { + // @ts-ignore + if (status[key]?.success) { + successCount[key] = successCount[key] + 1; + } + }); } if (status.transformJobDeleted?.error) { const error = extractErrorMessage(status.transformJobDeleted.error); - toastNotifications.addDanger( - i18n.translate('xpack.transform.transformList.deleteTransformErrorMessage', { + toastNotifications.addDanger({ + title: i18n.translate('xpack.transform.transformList.deleteTransformErrorMessage', { defaultMessage: - 'An error occurred deleting the data frame analytics job {transformId}: {error}', - values: { transformId, error }, - }) - ); + 'An error occurred deleting the data frame analytics job {transformId}', + values: { transformId }, + }), + text: toMountPoint(), + }); } - if (status.destIndexDeleted?.success) { - toastNotifications.addSuccess( - i18n.translate( - 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexSuccessMessage', - { - defaultMessage: - 'Request to delete destination index {destinationIndex} acknowledged.', - values: { destinationIndex }, - } - ) - ); - } if (status.destIndexDeleted?.error) { const error = extractErrorMessage(status.destIndexDeleted.error); - toastNotifications.addDanger( - i18n.translate( + toastNotifications.addDanger({ + title: i18n.translate( 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexErrorMessage', { - defaultMessage: - 'An error occurred deleting destination index {destinationIndex}: {error}', - values: { destinationIndex, error }, - } - ) - ); - } - - if (status.destIndexPatternDeleted?.success) { - toastNotifications.addSuccess( - i18n.translate( - 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternSuccessMessage', - { - defaultMessage: - 'Request to delete index pattern {destinationIndex} acknowledged.', + defaultMessage: 'An error occurred deleting destination index {destinationIndex}', values: { destinationIndex }, } - ) - ); + ), + text: toMountPoint(), + }); } + if (status.destIndexPatternDeleted?.error) { const error = extractErrorMessage(status.destIndexPatternDeleted.error); - toastNotifications.addDanger( - i18n.translate( + toastNotifications.addDanger({ + title: i18n.translate( 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternErrorMessage', { - defaultMessage: - 'An error occurred deleting index pattern {destinationIndex}: {error}', - values: { destinationIndex, error }, + defaultMessage: 'An error occurred deleting index pattern {destinationIndex}', + values: { destinationIndex }, } - ) - ); + ), + text: toMountPoint(), + }); } } } + if (isBulk) { + if (successCount.transformJobDeleted > 0) { + toastNotifications.addSuccess( + i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { + defaultMessage: + 'Successfully deleted {count} transform {count, plural, one {job} other {jobs}}.', + values: { count: successCount.transformJobDeleted }, + }) + ); + } + + if (successCount.destIndexDeleted > 0) { + toastNotifications.addSuccess( + i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { + defaultMessage: + 'Successfully deleted {count} destination {count, plural, one {index} other {indices}}.', + values: { count: successCount.destIndexDeleted }, + }) + ); + } + if (successCount.destIndexPatternDeleted > 0) { + toastNotifications.addSuccess( + i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { + defaultMessage: + 'Successfully deleted {count} destination index {count, plural, one {pattern} other {patterns}}.', + values: { count: successCount.destIndexPatternDeleted }, + }) + ); + } + } + refreshTransformList$.next(REFRESH_TRANSFORM_LIST_STATE.REFRESH); } catch (e) { toastNotifications.addDanger({ diff --git a/x-pack/plugins/transform/server/routes/api/transforms.ts b/x-pack/plugins/transform/server/routes/api/transforms.ts index 9dc3cac2306cc..265ff7bf232f1 100644 --- a/x-pack/plugins/transform/server/routes/api/transforms.ts +++ b/x-pack/plugins/transform/server/routes/api/transforms.ts @@ -435,7 +435,7 @@ async function deleteTransformsWithDestIndex( if (userCanDeleteDestIndex) { try { await ctx.transform!.dataClient.callAsCurrentUser('indices.delete', { - index: destinationIndex, + index: 'blah_dne', }); destIndexDeleted.success = true; } catch (deleteIndexError) { @@ -455,8 +455,8 @@ async function deleteTransformsWithDestIndex( ); if (indexPatternId) { await deleteDestIndexPatternById(indexPatternId, ctx.core.savedObjects.client); + destIndexPatternDeleted.success = true; } - destIndexPatternDeleted.success = true; } catch (deleteDestIndexPatternError) { destIndexPatternDeleted.error = wrapError(deleteDestIndexPatternError); } From f2d672b51f6ea6546abc0dcf7b21b40b1f2b11d1 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 2 Jun 2020 15:36:11 -0500 Subject: [PATCH 03/17] [Transform] Change preview text length to 50 --- .../public/app/hooks/use_delete_transform.tsx | 20 +++++++++++++++---- .../transform/server/routes/api/transforms.ts | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index e6136e5d71873..5a4b679fda335 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -238,7 +238,9 @@ export const useDeleteTransforms = () => { 'An error occurred deleting the data frame analytics job {transformId}', values: { transformId }, }), - text: toMountPoint(), + text: toMountPoint( + + ), }); } @@ -252,7 +254,9 @@ export const useDeleteTransforms = () => { values: { destinationIndex }, } ), - text: toMountPoint(), + text: toMountPoint( + + ), }); } @@ -266,7 +270,9 @@ export const useDeleteTransforms = () => { values: { destinationIndex }, } ), - text: toMountPoint(), + text: toMountPoint( + + ), }); } } @@ -309,7 +315,13 @@ export const useDeleteTransforms = () => { title: i18n.translate('xpack.transform.transformList.deleteTransformGenericErrorMessage', { defaultMessage: 'An error occurred calling the API endpoint to delete transforms.', }), - text: toMountPoint(), + text: toMountPoint( + + ), }); } }; diff --git a/x-pack/plugins/transform/server/routes/api/transforms.ts b/x-pack/plugins/transform/server/routes/api/transforms.ts index 265ff7bf232f1..acabed0e6bb1f 100644 --- a/x-pack/plugins/transform/server/routes/api/transforms.ts +++ b/x-pack/plugins/transform/server/routes/api/transforms.ts @@ -435,7 +435,7 @@ async function deleteTransformsWithDestIndex( if (userCanDeleteDestIndex) { try { await ctx.transform!.dataClient.callAsCurrentUser('indices.delete', { - index: 'blah_dne', + index: destinationIndex, }); destIndexDeleted.success = true; } catch (deleteIndexError) { From 1e6bd0ccb78f5fa5d93d8747eb946f0f215e9ec9 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Wed, 3 Jun 2020 16:50:12 -0500 Subject: [PATCH 04/17] [Transform] Add API integration tests for delete_transforms --- .../transform/server/routes/api/transforms.ts | 3 + x-pack/test/api_integration/apis/index.js | 1 + .../apis/ml/data_frame_analytics/delete.ts | 2 +- .../apis/transform/delete_transforms.ts | 197 ++++++++++++++++++ .../api_integration/apis/transform/index.ts | 32 +++ x-pack/test/api_integration/services/index.ts | 2 + .../api_integration/services/transform.ts | 23 ++ .../test/functional/services/transform/api.ts | 66 ++++-- 8 files changed, 313 insertions(+), 13 deletions(-) create mode 100644 x-pack/test/api_integration/apis/transform/delete_transforms.ts create mode 100644 x-pack/test/api_integration/apis/transform/index.ts create mode 100644 x-pack/test/api_integration/services/transform.ts diff --git a/x-pack/plugins/transform/server/routes/api/transforms.ts b/x-pack/plugins/transform/server/routes/api/transforms.ts index acabed0e6bb1f..0ad67c7b48259 100644 --- a/x-pack/plugins/transform/server/routes/api/transforms.ts +++ b/x-pack/plugins/transform/server/routes/api/transforms.ts @@ -472,6 +472,9 @@ async function deleteTransformsWithDestIndex( if (transformJobDeleted.error.statusCode === 404) { return response.notFound(); } + if (transformJobDeleted.error.statusCode === 403) { + return response.forbidden(); + } } results[transformId] = { diff --git a/x-pack/test/api_integration/apis/index.js b/x-pack/test/api_integration/apis/index.js index f3a2d2fa3cd8f..ba97bd191edf2 100644 --- a/x-pack/test/api_integration/apis/index.js +++ b/x-pack/test/api_integration/apis/index.js @@ -30,5 +30,6 @@ export default function ({ loadTestFile }) { loadTestFile(require.resolve('./ingest_manager')); loadTestFile(require.resolve('./endpoint')); loadTestFile(require.resolve('./ml')); + loadTestFile(require.resolve('./transform')); }); } diff --git a/x-pack/test/api_integration/apis/ml/data_frame_analytics/delete.ts b/x-pack/test/api_integration/apis/ml/data_frame_analytics/delete.ts index 23bff0d0c2855..dc0ccfdc53a18 100644 --- a/x-pack/test/api_integration/apis/ml/data_frame_analytics/delete.ts +++ b/x-pack/test/api_integration/apis/ml/data_frame_analytics/delete.ts @@ -197,7 +197,7 @@ export default ({ getService }: FtrProviderContext) => { await ml.testResources.deleteIndexPattern(destinationIndex); }); - it('deletes job, target index, and index pattern by id', async () => { + it('should delete job, target index, and index pattern by id', async () => { const { body } = await supertest .delete(`/api/ml/data_frame/analytics/${analyticsId}`) .query({ deleteDestIndex: true, deleteDestIndexPattern: true }) diff --git a/x-pack/test/api_integration/apis/transform/delete_transforms.ts b/x-pack/test/api_integration/apis/transform/delete_transforms.ts new file mode 100644 index 0000000000000..de512779323b0 --- /dev/null +++ b/x-pack/test/api_integration/apis/transform/delete_transforms.ts @@ -0,0 +1,197 @@ +/* + * 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 expect from '@kbn/expect'; +import { TransformEndpointRequest } from '../../../../plugins/transform/common'; +import { FtrProviderContext } from '../../ftr_provider_context'; +import { COMMON_REQUEST_HEADERS } from '../../../functional/services/ml/common'; +import { USER } from '../../../functional/services/transform/security_common'; + +// eslint-disable-next-line import/no-default-export +export default ({ getService }: FtrProviderContext) => { + const esArchiver = getService('esArchiver'); + const supertest = getService('supertestWithoutAuth'); + const transform = getService('transform'); + + function generateDestIndex(transformId: string): string { + return `dest_del_${transformId}`; + } + + async function createTransformJob(transformId: string, destinationIndex: string) { + const config = { + id: transformId, + source: { index: ['farequote-*'] }, + pivot: { + group_by: { airline: { terms: { field: 'airline' } } }, + aggregations: { '@timestamp.value_count': { value_count: { field: '@timestamp' } } }, + }, + dest: { index: destinationIndex }, + }; + + await transform.api.createTransform(config); + } + + describe('delete_transforms', function () { + before(async () => { + await esArchiver.loadIfNeeded('ml/farequote'); + await transform.testResources.setKibanaTimeZoneToUTC(); + }); + + after(async () => { + await transform.api.cleanTransformIndices(); + }); + + describe('single deletion', function () { + const transformId = 'test2'; + const destinationIndex = generateDestIndex(transformId); + before(async () => { + await createTransformJob(transformId, destinationIndex); + await transform.api.createIndices(destinationIndex); + }); + + after(async () => { + await transform.api.deleteIndices(destinationIndex); + }); + + it('should delete transform job by transformId', async () => { + const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; + const { body } = await supertest + .post(`/api/transform/delete_transforms`) + .auth( + USER.TRANSFORM_POWERUSER, + transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) + ) + .set(COMMON_REQUEST_HEADERS) + .send({ + transformsInfo, + }) + .expect(200); + + expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].destIndexDeleted.success).to.eql(false); + expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); + await transform.api.waitForIndicesToExist(destinationIndex); + await transform.api.waitForTransformJobNotToExist(transformId); + }); + + it('should return response of status 200 with error info if invalid transformId', async () => { + const transformsInfo: TransformEndpointRequest[] = [{ id: 'invalid_trasnform_id' }]; + await supertest + .post(`/api/transform/delete_transforms`) + .auth( + USER.TRANSFORM_VIEWER, + transform.securityCommon.getPasswordForUser(USER.TRANSFORM_VIEWER) + ) + .set(COMMON_REQUEST_HEADERS) + .send({ + transformsInfo, + }); + // .expect(200); + + // expect(body.invalid_trasnform_id.transformJobDeleted.success).to.eql(false); + // expect(body.invalid_trasnform_id.transformJobDeleted.error).not.to.eql(undefined); + }); + }); + // + // describe('bulk deletion', function () { + // const transformsInfo: TransformEndpointRequest[] = [ + // { id: 'bulk_delete_test_1' }, + // { id: 'bulk_delete_test_2' }, + // ]; + // + // before(async () => { + // await createTransformJob('bulk_delete_test_1', 'bulk_delete_test_1'); + // await transform.api.createIndices('bulk_delete_test_1'); + // await createTransformJob('bulk_delete_test_2', 'bulk_delete_test_2'); + // await transform.api.createIndices('bulk_delete_test_2'); + // }); + // + // after(async () => { + // await transform.api.deleteIndices('bulk_delete_test_1'); + // await transform.api.deleteIndices('bulk_delete_test_2'); + // }); + // + // it('should delete multiple transform jobs by transformIds', async () => { + // const { body } = await supertest + // .post(`/api/transform/delete_transforms`) + // .auth( + // USER.TRANSFORM_POWERUSER, + // transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) + // ) + // .set(COMMON_REQUEST_HEADERS) + // .send({ + // transformsInfo, + // }) + // .expect(200); + // + // transformsInfo.forEach(({ id: transformId }) => { + // expect(body[transformId].transformJobDeleted.success).to.eql(true); + // expect(body[transformId].destIndexDeleted).to.eql(undefined); + // expect(body[transformId].destIndexPatternDeleted).to.eql(undefined); + // }); + // await transform.api.waitForTransformJobNotToExist('bulk_delete_test_1'); + // await transform.api.waitForTransformJobNotToExist('bulk_delete_test_2'); + // }); + // }); + // + // describe('with deleteDestIndex', function () { + // const transformId = 'test2'; + // const destinationIndex = generateDestIndex(transformId); + // + // before(async () => { + // await createTransformJob(transformId, destinationIndex); + // await transform.api.createIndices(destinationIndex); + // }); + // + // after(async () => { + // await transform.api.deleteIndices(destinationIndex); + // }); + // + // it('should delete transform and destination index', async () => { + // const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; + // const { body } = await supertest + // .post(`/api/transform/delete_transforms`) + // .auth( + // USER.TRANSFORM_POWERUSER, + // transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) + // ) + // .set(COMMON_REQUEST_HEADERS) + // .send({ + // transformsInfo, + // deleteDestIndex: true, + // }) + // .expect(200); + // + // expect(body[transformId].transformJobDeleted.success).to.eql(true); + // expect(body[transformId].destIndexDeleted.success).to.eql(true); + // expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); + // await transform.api.waitForTransformJobNotToExist(destinationIndex); + // await transform.api.waitForIndicesNotToExist(destinationIndex); + // }); + // it('should return 404 for unauthorized user', async () => { + // const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; + // const { body } = await supertest + // .post(`/api/transform/delete_transforms`) + // .auth( + // USER.TRANSFORM_VIEWER, + // transform.securityCommon.getPasswordForUser(USER.TRANSFORM_VIEWER) + // ) + // .set(COMMON_REQUEST_HEADERS) + // .send({ + // transformsInfo, + // deleteDestIndex: true, + // }) + // .expect(200); + // + // console.log('BODY', body); + // + // // expect(body.error).to.eql('Not Found'); + // // expect(body.message).to.eql('Not Found'); + // // await transform.api.waitForTransformJobToExist(destinationIndex); + // // await transform.api.waitForIndicesToExist(destinationIndex); + // }); + // }); + }); +}; diff --git a/x-pack/test/api_integration/apis/transform/index.ts b/x-pack/test/api_integration/apis/transform/index.ts new file mode 100644 index 0000000000000..93a951a55ece1 --- /dev/null +++ b/x-pack/test/api_integration/apis/transform/index.ts @@ -0,0 +1,32 @@ +/* + * 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 { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ getService, loadTestFile }: FtrProviderContext) { + const esArchiver = getService('esArchiver'); + const transform = getService('transform'); + + describe('Machine Learning', function () { + this.tags(['transform']); + + before(async () => { + await transform.securityCommon.createTransformRoles(); + await transform.securityCommon.createTransformUsers(); + }); + + after(async () => { + await transform.securityCommon.cleanTransformUsers(); + await transform.securityCommon.cleanTransformRoles(); + + await esArchiver.unload('ml/farequote'); + + await transform.testResources.resetKibanaTimeZone(); + }); + + loadTestFile(require.resolve('./delete_transforms')); + }); +} diff --git a/x-pack/test/api_integration/services/index.ts b/x-pack/test/api_integration/services/index.ts index 687984340d7d6..b084845ea37b8 100644 --- a/x-pack/test/api_integration/services/index.ts +++ b/x-pack/test/api_integration/services/index.ts @@ -25,6 +25,7 @@ import { InfraLogSourceConfigurationProvider } from './infra_log_source_configur import { MachineLearningProvider } from './ml'; import { IngestManagerProvider } from './ingest_manager'; import { ResolverGeneratorProvider } from './resolver'; +import { TransformProvider } from './transform'; export const services = { ...commonServices, @@ -45,4 +46,5 @@ export const services = { ml: MachineLearningProvider, ingestManager: IngestManagerProvider, resolverGenerator: ResolverGeneratorProvider, + transform: TransformProvider, }; diff --git a/x-pack/test/api_integration/services/transform.ts b/x-pack/test/api_integration/services/transform.ts new file mode 100644 index 0000000000000..1403d5d2d67f0 --- /dev/null +++ b/x-pack/test/api_integration/services/transform.ts @@ -0,0 +1,23 @@ +/* + * 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 { FtrProviderContext } from '../../functional/ftr_provider_context'; + +import { TransformAPIProvider } from '../../functional/services/transform/api'; +import { TransformSecurityCommonProvider } from '../../functional/services/transform/security_common'; +import { MachineLearningTestResourcesProvider } from '../../functional/services/ml/test_resources'; + +export function TransformProvider(context: FtrProviderContext) { + const api = TransformAPIProvider(context); + const securityCommon = TransformSecurityCommonProvider(context); + const testResources = MachineLearningTestResourcesProvider(context); + + return { + api, + securityCommon, + testResources, + }; +} diff --git a/x-pack/test/functional/services/transform/api.ts b/x-pack/test/functional/services/transform/api.ts index a805f5a3b6013..fc56559bc34f0 100644 --- a/x-pack/test/functional/services/transform/api.ts +++ b/x-pack/test/functional/services/transform/api.ts @@ -20,9 +20,24 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { const esSupertest = getService('esSupertest'); return { + async createIndices(indices: string) { + log.debug(`Creating indices: '${indices}'...`); + if (await es.indices.exists({ index: indices, allowNoIndices: false })) { + log.debug(`Indices '${indices}' already exist. Nothing to create.`); + return; + } + + const createResponse = await es.indices.create({ index: indices }); + expect(createResponse) + .to.have.property('acknowledged') + .eql(true, 'Response for create request indices should be acknowledged.'); + + await this.waitForIndicesToExist(indices); + }, + async deleteIndices(indices: string) { log.debug(`Deleting indices: '${indices}'...`); - if ((await es.indices.exists({ index: indices, allowNoIndices: false })) === false) { + if (!(await es.indices.exists({ index: indices, allowNoIndices: false }))) { log.debug(`Indices '${indices}' don't exist. Nothing to delete.`); return; } @@ -34,11 +49,25 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { .to.have.property('acknowledged') .eql(true, 'Response for delete request should be acknowledged'); - await retry.waitForWithTimeout(`'${indices}' indices to be deleted`, 30 * 1000, async () => { - if ((await es.indices.exists({ index: indices, allowNoIndices: false })) === false) { + await this.waitForIndicesNotToExist(indices, `expected indices '${indices}' to be deleted`); + }, + + async waitForIndicesToExist(indices: string) { + await retry.tryForTime(30 * 1000, async () => { + if (await es.indices.exists({ index: indices, allowNoIndices: false })) { return true; } else { - throw new Error(`expected indices '${indices}' to be deleted`); + throw new Error(`indices '${indices}' should exist`); + } + }); + }, + + async waitForIndicesNotToExist(indices: string, errorMsg?: string) { + await retry.tryForTime(30 * 1000, async () => { + if (!(await es.indices.exists({ index: indices, allowNoIndices: false }))) { + return true; + } else { + throw new Error(errorMsg || `indices '${indices}' should not exist`); } }); }, @@ -63,9 +92,7 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { async getTransformState(transformId: string): Promise { const stats = await this.getTransformStats(transformId); - const state: TRANSFORM_STATE = stats.state; - - return state; + return stats.state; }, async waitForTransformState(transformId: string, expectedState: TRANSFORM_STATE) { @@ -96,8 +123,8 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { }); }, - async getTransform(transformId: string) { - return await esSupertest.get(`/_transform/${transformId}`).expect(200); + async getTransform(transformId: string, expectedCode = 200) { + return await esSupertest.get(`/_transform/${transformId}`).expect(expectedCode); }, async createTransform(transformConfig: TransformPivotConfig) { @@ -105,11 +132,26 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { log.debug(`Creating transform with id '${transformId}'...`); await esSupertest.put(`/_transform/${transformId}`).send(transformConfig).expect(200); - await retry.waitForWithTimeout(`'${transformId}' to be created`, 5 * 1000, async () => { - if (await this.getTransform(transformId)) { + await this.waitForTransformJobToExist( + transformId, + `expected transform '${transformId}' to be created` + ); + }, + async waitForTransformJobToExist(transformId: string, errorMsg?: string) { + await retry.waitForWithTimeout(`'${transformId}' to exist`, 5 * 1000, async () => { + if (await this.getTransform(transformId, 200)) { + return true; + } else { + throw new Error(errorMsg || `expected transform '${transformId}' to exist`); + } + }); + }, + async waitForTransformJobNotToExist(transformId: string, errorMsg?: string) { + await retry.waitForWithTimeout(`'${transformId}' to exist`, 5 * 1000, async () => { + if (await this.getTransform(transformId, 404)) { return true; } else { - throw new Error(`expected transform '${transformId}' to be created`); + throw new Error(errorMsg || `expected transform '${transformId}' to not exist`); } }); }, From b32b8842804afc11446d7d00e616db9c0f555af9 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Wed, 3 Jun 2020 21:05:53 -0500 Subject: [PATCH 05/17] [Transform] Update tests and fix i18n duplicates --- x-pack/plugins/ml/common/util/errors.ts | 33 ++ .../ml/public/application/util/error_utils.ts | 7 + x-pack/plugins/ml/public/shared.ts | 1 + .../public/app/hooks/use_delete_transform.tsx | 67 +--- .../transform_list/action_delete.tsx | 8 +- .../transform/public/shared_imports.ts | 1 + .../transform/server/routes/api/transforms.ts | 150 +++------ .../apis/transform/delete_transforms.ts | 286 +++++++++++------- .../test/functional/services/transform/api.ts | 95 +++++- .../services/transform/security_common.ts | 2 +- 10 files changed, 369 insertions(+), 281 deletions(-) diff --git a/x-pack/plugins/ml/common/util/errors.ts b/x-pack/plugins/ml/common/util/errors.ts index 4446624bf2e7f..6f620f74a4531 100644 --- a/x-pack/plugins/ml/common/util/errors.ts +++ b/x-pack/plugins/ml/common/util/errors.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; import { isErrorResponse } from '../types/errors'; export function getErrorMessage(error: any) { @@ -17,3 +18,35 @@ export function getErrorMessage(error: any) { return JSON.stringify(error); } + +export const extractErrorMessage = ( + error: CustomHttpResponseOptions | undefined | string +): string | undefined => { + if (typeof error === 'string') { + return error; + } + + if (error?.body) { + if (typeof error.body === 'string') { + return error.body; + } + if (typeof error.body === 'object' && 'message' in error.body) { + if (typeof error.body.message === 'string') { + return error.body.message; + } + // @ts-ignore + if (typeof (error.body.message?.msg === 'string')) { + // @ts-ignore + return error.body.message?.msg; + } + } + if (typeof error.body === 'object' && 'msg' in error.body) { + // @ts-ignore + if (typeof error.body.msg === 'string') { + // @ts-ignore + return error.body.msg; + } + } + } + return undefined; +}; diff --git a/x-pack/plugins/ml/public/application/util/error_utils.ts b/x-pack/plugins/ml/public/application/util/error_utils.ts index 2ce8f4ffc583a..427bf973d44dd 100644 --- a/x-pack/plugins/ml/public/application/util/error_utils.ts +++ b/x-pack/plugins/ml/public/application/util/error_utils.ts @@ -27,6 +27,13 @@ export const extractErrorMessage = ( return error.body.message?.msg; } } + if (typeof error.body === 'object' && 'msg' in error.body) { + // @ts-ignore + if (typeof error.body.msg === 'string') { + // @ts-ignore + return error.body.msg; + } + } } return undefined; }; diff --git a/x-pack/plugins/ml/public/shared.ts b/x-pack/plugins/ml/public/shared.ts index 6821cb7ef0f94..ff83d79adff67 100644 --- a/x-pack/plugins/ml/public/shared.ts +++ b/x-pack/plugins/ml/public/shared.ts @@ -14,6 +14,7 @@ export * from '../common/types/audit_message'; export * from '../common/util/anomaly_utils'; export * from '../common/util/errors'; + export * from '../common/util/validators'; export * from './application/formatters/metric_change_description'; diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index 5a4b679fda335..e553b6c00eed0 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -5,56 +5,18 @@ */ import React, { useCallback, useEffect, useState } from 'react'; - import { i18n } from '@kbn/i18n'; -import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; import { CoreSetup } from 'kibana/public'; import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public'; - import { TransformEndpointRequest, DeleteTransformEndpointResult } from '../../../common'; - -import { getErrorMessage } from '../../shared_imports'; - +import { getErrorMessage, extractErrorMessage } from '../../shared_imports'; import { useAppDependencies, useToastNotifications } from '../app_dependencies'; import { TransformListRow, refreshTransformList$, REFRESH_TRANSFORM_LIST_STATE } from '../common'; import { ToastNotificationText } from '../components'; - import { useApi } from './use_api'; import { API_BASE_PATH } from '../../../common/constants'; import { IIndexPattern } from '../../../../../../src/plugins/data/common/index_patterns'; -export const extractErrorMessage = ( - error: CustomHttpResponseOptions | undefined | string -): string | undefined => { - if (typeof error === 'string') { - return error; - } - - if (error?.body) { - if (typeof error.body === 'string') { - return error.body; - } - if (typeof error.body === 'object' && 'message' in error.body) { - if (typeof error.body.message === 'string') { - return error.body.message; - } - // @ts-ignore - if (typeof (error.body.message?.msg === 'string')) { - // @ts-ignore - return error.body.message?.msg; - } - } - if (typeof error.body === 'object' && 'msg' in error.body) { - // @ts-ignore - if (typeof error.body.msg === 'string') { - // @ts-ignore - return error.body.msg; - } - } - } - return undefined; -}; - export const canDeleteIndex = async (http: CoreSetup['http']) => { const privilege = await http.get(`${API_BASE_PATH}privileges`); if (!privilege) { @@ -100,7 +62,7 @@ export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { toastNotifications.addDanger( i18n.translate( - 'xpack.ml.dataframe.analyticsList.errorWithCheckingIfIndexPatternExistsNotificationErrorMessage', + 'xpack.transform.deleteTransfrom.errorWithCheckingIfIndexPatternExistsNotificationErrorMessage', { defaultMessage: 'An error occurred checking if index pattern {indexPattern} exists: {error}', @@ -201,7 +163,7 @@ export const useDeleteTransforms = () => { if (status.destIndexDeleted?.success) { toastNotifications.addSuccess( i18n.translate( - 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexSuccessMessage', + 'xpack.transform.deleteTransfrom.deleteAnalyticsWithIndexSuccessMessage', { defaultMessage: 'Request to delete destination index {destinationIndex} acknowledged.', @@ -213,7 +175,7 @@ export const useDeleteTransforms = () => { if (status.destIndexPatternDeleted?.success) { toastNotifications.addSuccess( i18n.translate( - 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternSuccessMessage', + 'xpack.transform.deleteTransfrom.deleteAnalyticsWithIndexPatternSuccessMessage', { defaultMessage: 'Request to delete index pattern {destinationIndex} acknowledged.', @@ -248,7 +210,7 @@ export const useDeleteTransforms = () => { const error = extractErrorMessage(status.destIndexDeleted.error); toastNotifications.addDanger({ title: i18n.translate( - 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexErrorMessage', + 'xpack.transform.deleteTransfrom.deleteAnalyticsWithIndexErrorMessage', { defaultMessage: 'An error occurred deleting destination index {destinationIndex}', values: { destinationIndex }, @@ -264,7 +226,7 @@ export const useDeleteTransforms = () => { const error = extractErrorMessage(status.destIndexPatternDeleted.error); toastNotifications.addDanger({ title: i18n.translate( - 'xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternErrorMessage', + 'xpack.transform.deleteTransfrom.deleteAnalyticsWithIndexPatternErrorMessage', { defaultMessage: 'An error occurred deleting index pattern {destinationIndex}', values: { destinationIndex }, @@ -281,7 +243,7 @@ export const useDeleteTransforms = () => { if (isBulk) { if (successCount.transformJobDeleted > 0) { toastNotifications.addSuccess( - i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { + i18n.translate('xpack.transform.transformList.bulkDeleteTransformSuccessMessage', { defaultMessage: 'Successfully deleted {count} transform {count, plural, one {job} other {jobs}}.', values: { count: successCount.transformJobDeleted }, @@ -291,7 +253,7 @@ export const useDeleteTransforms = () => { if (successCount.destIndexDeleted > 0) { toastNotifications.addSuccess( - i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { + i18n.translate('xpack.transform.transformList.bulkDeleteDestIndexSuccessMessage', { defaultMessage: 'Successfully deleted {count} destination {count, plural, one {index} other {indices}}.', values: { count: successCount.destIndexDeleted }, @@ -300,11 +262,14 @@ export const useDeleteTransforms = () => { } if (successCount.destIndexPatternDeleted > 0) { toastNotifications.addSuccess( - i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { - defaultMessage: - 'Successfully deleted {count} destination index {count, plural, one {pattern} other {patterns}}.', - values: { count: successCount.destIndexPatternDeleted }, - }) + i18n.translate( + 'xpack.transform.transformList.bulkDeleteDestIndexPatternSuccessMessage', + { + defaultMessage: + 'Successfully deleted {count} destination index {count, plural, one {pattern} other {patterns}}.', + values: { count: successCount.destIndexPatternDeleted }, + } + ) ); } } diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx index 3a4ad38b79c21..50f46db8ee705 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx @@ -101,7 +101,7 @@ export const DeleteAction: FC = ({ items, forceDisable }) => data-test-subj="mlAnalyticsJobDeleteIndexSwitch" style={{ paddingBottom: 10 }} label={i18n.translate( - 'xpack.ml.dataframe.analyticsList.deleteDestinationIndexTitle', + 'xpack.transform.actionDeleteTransform.bulkDeleteDestinationIndexTitle', { defaultMessage: 'Delete destination index', } @@ -116,7 +116,7 @@ export const DeleteAction: FC = ({ items, forceDisable }) => = ({ items, forceDisable }) => data-test-subj="mlAnalyticsJobDeleteIndexSwitch" style={{ paddingBottom: 10 }} label={i18n.translate( - 'xpack.ml.dataframe.analyticsList.deleteDestinationIndexTitle', + 'xpack.transform.actionDeleteTransform.deleteDestinationIndexTitle', { defaultMessage: 'Delete destination index {destinationIndex}', values: { destinationIndex: items[0] && items[0].config.dest.index }, @@ -161,7 +161,7 @@ export const DeleteAction: FC = ({ items, forceDisable }) => { - if (!license.getStatus().isSecurityEnabled) { - // If security isn't enabled, let the user use app. - return true; - } - - const { has_all_requested: hasAllPrivileges } = await callAsCurrentUser('transport.request', { - path: '/_security/user/_has_privileges', - method: 'POST', - body: { - index: [ - { - names: [indexName], - privileges: ['delete_index'], - }, - ], - }, - }); - return hasAllPrivileges; -} - export function registerTransformsRoutes(routeDependencies: RouteDependencies) { const { router, license } = routeDependencies; router.get( @@ -226,25 +200,27 @@ export function registerTransformsRoutes(routeDependencies: RouteDependencies) { } = req.body as DeleteTransformEndpoint; try { - if (deleteDestIndex || deleteDestIndexPattern) { - return res.ok({ - body: await deleteTransformsWithDestIndex( - transformsInfo, - deleteDestIndex, - deleteDestIndexPattern, - ctx, - license, - res - ), - }); - } else { - return res.ok({ - body: await deleteTransforms( - transformsInfo, - ctx.transform!.dataClient.callAsCurrentUser - ), - }); + const body = await deleteTransforms( + transformsInfo, + deleteDestIndex, + deleteDestIndexPattern, + ctx, + license, + res + ); + + if (body && body.status) { + if (body.status === 404) { + return res.notFound(); + } + if (body.status === 403) { + return res.forbidden(); + } } + + return res.ok({ + body, + }); } catch (e) { return res.customError(wrapError(wrapEsError(e))); } @@ -300,51 +276,6 @@ const getTransforms = async (options: { transformId?: string }, callAsCurrentUse return await callAsCurrentUser('transform.getTransforms', options); }; -async function deleteTransforms( - transformsInfo: TransformEndpointRequest[], - callAsCurrentUser: CallCluster -) { - const results: DeleteTransformEndpointResult = {}; - - for (const transformInfo of transformsInfo) { - const transformId = transformInfo.id; - try { - if (transformInfo.state === TRANSFORM_STATE.FAILED) { - try { - await callAsCurrentUser('transform.stopTransform', { - transformId, - force: true, - waitForCompletion: true, - } as StopOptions); - } catch (e) { - if (isRequestTimeout(e)) { - return fillResultsWithTimeouts({ - results, - id: transformId, - items: transformsInfo, - action: TRANSFORM_ACTIONS.DELETE, - }); - } - } - } - - await callAsCurrentUser('transform.deleteTransform', { transformId }); - results[transformId] = { transformJobDeleted: { success: true } }; - } catch (e) { - if (isRequestTimeout(e)) { - return fillResultsWithTimeouts({ - results, - id: transformInfo.id, - items: transformsInfo, - action: TRANSFORM_ACTIONS.DELETE, - }); - } - results[transformId] = { transformJobDeleted: { success: false, error: JSON.stringify(e) } }; - } - } - return results; -} - async function getIndexPatternId( indexName: string, savedObjectsClient: SavedObjectsClientContract @@ -371,7 +302,7 @@ async function deleteDestIndexPatternById( return await savedObjectsClient.delete('index-pattern', indexPatternId); } -async function deleteTransformsWithDestIndex( +async function deleteTransforms( transformsInfo: TransformEndpointRequest[], deleteDestIndex: boolean | undefined, deleteDestIndexPattern: boolean | undefined, @@ -420,29 +351,27 @@ async function deleteTransformsWithDestIndex( ? transformConfig.dest.index[0] : transformConfig.dest.index; } catch (getTransformConfigError) { - return response.customError(wrapError(getTransformConfigError)); + transformJobDeleted.error = wrapError(getTransformConfigError); + results[transformId] = { + transformJobDeleted, + destIndexDeleted, + destIndexPatternDeleted, + destinationIndex, + }; + continue; } // If user checks box to delete the destinationIndex associated with the job if (destinationIndex && deleteDestIndex) { - // Verify if user has privilege to delete the destination index - const userCanDeleteDestIndex = await canDeleteIndex( - destinationIndex, - ctx.transform!.dataClient.callAsCurrentUser, - license - ); - // If user does have privilege to delete the index, then delete the index - if (userCanDeleteDestIndex) { - try { - await ctx.transform!.dataClient.callAsCurrentUser('indices.delete', { - index: destinationIndex, - }); - destIndexDeleted.success = true; - } catch (deleteIndexError) { - destIndexDeleted.error = wrapError(deleteIndexError); - } - } else { - return response.forbidden(); + try { + // If user does have privilege to delete the index, then delete the index + // if no permission then return 403 forbidden + await ctx.transform!.dataClient.callAsCurrentUser('indices.delete', { + index: destinationIndex, + }); + destIndexDeleted.success = true; + } catch (deleteIndexError) { + destIndexDeleted.error = wrapError(deleteIndexError); } } @@ -469,9 +398,6 @@ async function deleteTransformsWithDestIndex( transformJobDeleted.success = true; } catch (deleteTransformJobError) { transformJobDeleted.error = wrapError(deleteTransformJobError); - if (transformJobDeleted.error.statusCode === 404) { - return response.notFound(); - } if (transformJobDeleted.error.statusCode === 403) { return response.forbidden(); } diff --git a/x-pack/test/api_integration/apis/transform/delete_transforms.ts b/x-pack/test/api_integration/apis/transform/delete_transforms.ts index de512779323b0..c282591621514 100644 --- a/x-pack/test/api_integration/apis/transform/delete_transforms.ts +++ b/x-pack/test/api_integration/apis/transform/delete_transforms.ts @@ -16,7 +16,7 @@ export default ({ getService }: FtrProviderContext) => { const transform = getService('transform'); function generateDestIndex(transformId: string): string { - return `dest_del_${transformId}`; + return `dest_${transformId}`; } async function createTransformJob(transformId: string, destinationIndex: string) { @@ -43,15 +43,16 @@ export default ({ getService }: FtrProviderContext) => { await transform.api.cleanTransformIndices(); }); - describe('single deletion', function () { - const transformId = 'test2'; + describe('single transform deletion', function () { + const transformId = 'test1'; const destinationIndex = generateDestIndex(transformId); - before(async () => { + + beforeEach(async () => { await createTransformJob(transformId, destinationIndex); await transform.api.createIndices(destinationIndex); }); - after(async () => { + afterEach(async () => { await transform.api.deleteIndices(destinationIndex); }); @@ -76,8 +77,8 @@ export default ({ getService }: FtrProviderContext) => { await transform.api.waitForTransformJobNotToExist(transformId); }); - it('should return response of status 200 with error info if invalid transformId', async () => { - const transformsInfo: TransformEndpointRequest[] = [{ id: 'invalid_trasnform_id' }]; + it('should return 403 for unauthorized user', async () => { + const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; await supertest .post(`/api/transform/delete_transforms`) .auth( @@ -87,111 +88,176 @@ export default ({ getService }: FtrProviderContext) => { .set(COMMON_REQUEST_HEADERS) .send({ transformsInfo, - }); - // .expect(200); + }) + .expect(403); + await transform.api.waitForIndicesToExist(destinationIndex); + await transform.api.waitForTransformJobToExist(transformId); + }); + }); + + describe('single transform deletion with invalid transformId', function () { + it('should return 404 with error message if invalid transformId', async () => { + const transformsInfo: TransformEndpointRequest[] = [{ id: 'invalid_transform_id' }]; + const { body } = await supertest + .post(`/api/transform/delete_transforms`) + .auth( + USER.TRANSFORM_POWERUSER, + transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) + ) + .set(COMMON_REQUEST_HEADERS) + .send({ + transformsInfo, + }) + .expect(200); + expect(body.invalid_transform_id.transformJobDeleted.success).to.eql(false); + }); + }); + + describe('bulk deletion', function () { + const transformsInfo: TransformEndpointRequest[] = [ + { id: 'bulk_delete_test_1' }, + { id: 'bulk_delete_test_2' }, + ]; + + beforeEach(async () => { + await createTransformJob('bulk_delete_test_1', 'bulk_delete_test_1'); + await transform.api.createIndices('bulk_delete_test_1'); + await createTransformJob('bulk_delete_test_2', 'bulk_delete_test_2'); + await transform.api.createIndices('bulk_delete_test_2'); + }); + + afterEach(async () => { + await transform.api.deleteIndices('bulk_delete_test_1'); + await transform.api.deleteIndices('bulk_delete_test_2'); + }); + + it('should delete multiple transform jobs by transformIds', async () => { + const { body } = await supertest + .post(`/api/transform/delete_transforms`) + .auth( + USER.TRANSFORM_POWERUSER, + transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) + ) + .set(COMMON_REQUEST_HEADERS) + .send({ + transformsInfo, + }) + .expect(200); + + transformsInfo.forEach(({ id: transformId }) => { + expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].destIndexDeleted.success).to.eql(false); + expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); + }); + await transform.api.waitForTransformJobNotToExist('bulk_delete_test_1'); + await transform.api.waitForTransformJobNotToExist('bulk_delete_test_2'); + }); + + it('should delete multiple transform jobs by transformIds, even if one of the transformIds is invalid', async () => { + const invalidTransformId = 'invalid_transform_id'; + const { body } = await supertest + .post(`/api/transform/delete_transforms`) + .auth( + USER.TRANSFORM_POWERUSER, + transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) + ) + .set(COMMON_REQUEST_HEADERS) + .send({ + transformsInfo: [ + { id: 'bulk_delete_test_1' }, + { id: invalidTransformId }, + { id: 'bulk_delete_test_2' }, + ], + }) + .expect(200); + + transformsInfo.forEach(({ id: transformId }) => { + expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].destIndexDeleted.success).to.eql(false); + expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); + }); + await transform.api.waitForTransformJobNotToExist('bulk_delete_test_1'); + await transform.api.waitForTransformJobNotToExist('bulk_delete_test_2'); + expect(body[invalidTransformId].transformJobDeleted.success).to.eql(false); + expect(body[invalidTransformId].transformJobDeleted).to.have.property('error'); + }); + }); + + describe('with deleteDestIndex setting', function () { + const transformId = 'test2'; + const destinationIndex = generateDestIndex(transformId); + + before(async () => { + await createTransformJob(transformId, destinationIndex); + await transform.api.createIndices(destinationIndex); + }); + + after(async () => { + await transform.api.deleteIndices(destinationIndex); + }); + + it('should delete transform and destination index', async () => { + const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; + const { body } = await supertest + .post(`/api/transform/delete_transforms`) + .auth( + USER.TRANSFORM_POWERUSER, + transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) + ) + .set(COMMON_REQUEST_HEADERS) + .send({ + transformsInfo, + deleteDestIndex: true, + }) + .expect(200); + + expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].destIndexDeleted.success).to.eql(true); + expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); + await transform.api.waitForTransformJobNotToExist(transformId); + await transform.api.waitForIndicesNotToExist(destinationIndex); + }); + }); + + describe('with deleteDestIndexPattern setting', function () { + const transformId = 'test3'; + const destinationIndex = generateDestIndex(transformId); + + before(async () => { + await createTransformJob(transformId, destinationIndex); + await transform.api.createIndices(destinationIndex); + await transform.api.waitForIndicesToExist(destinationIndex); + await transform.api.createIndexPatternIfNeeded(destinationIndex); + }); + + after(async () => { + await transform.api.deleteIndices(destinationIndex); + await transform.api.deleteIndexPattern(destinationIndex); + }); + + it('should delete transform and destination index pattern', async () => { + const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; + const { body } = await supertest + .post(`/api/transform/delete_transforms`) + .auth( + USER.TRANSFORM_POWERUSER, + transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) + ) + .set(COMMON_REQUEST_HEADERS) + .send({ + transformsInfo, + deleteDestIndex: false, + deleteDestIndexPattern: true, + }) + .expect(200); - // expect(body.invalid_trasnform_id.transformJobDeleted.success).to.eql(false); - // expect(body.invalid_trasnform_id.transformJobDeleted.error).not.to.eql(undefined); + expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].destIndexDeleted.success).to.eql(false); + expect(body[transformId].destIndexPatternDeleted.success).to.eql(true); + await transform.api.waitForTransformJobNotToExist(transformId); + await transform.api.waitForIndicesToExist(destinationIndex); + await transform.api.waitForIndexPatternNotToExist(destinationIndex); }); }); - // - // describe('bulk deletion', function () { - // const transformsInfo: TransformEndpointRequest[] = [ - // { id: 'bulk_delete_test_1' }, - // { id: 'bulk_delete_test_2' }, - // ]; - // - // before(async () => { - // await createTransformJob('bulk_delete_test_1', 'bulk_delete_test_1'); - // await transform.api.createIndices('bulk_delete_test_1'); - // await createTransformJob('bulk_delete_test_2', 'bulk_delete_test_2'); - // await transform.api.createIndices('bulk_delete_test_2'); - // }); - // - // after(async () => { - // await transform.api.deleteIndices('bulk_delete_test_1'); - // await transform.api.deleteIndices('bulk_delete_test_2'); - // }); - // - // it('should delete multiple transform jobs by transformIds', async () => { - // const { body } = await supertest - // .post(`/api/transform/delete_transforms`) - // .auth( - // USER.TRANSFORM_POWERUSER, - // transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) - // ) - // .set(COMMON_REQUEST_HEADERS) - // .send({ - // transformsInfo, - // }) - // .expect(200); - // - // transformsInfo.forEach(({ id: transformId }) => { - // expect(body[transformId].transformJobDeleted.success).to.eql(true); - // expect(body[transformId].destIndexDeleted).to.eql(undefined); - // expect(body[transformId].destIndexPatternDeleted).to.eql(undefined); - // }); - // await transform.api.waitForTransformJobNotToExist('bulk_delete_test_1'); - // await transform.api.waitForTransformJobNotToExist('bulk_delete_test_2'); - // }); - // }); - // - // describe('with deleteDestIndex', function () { - // const transformId = 'test2'; - // const destinationIndex = generateDestIndex(transformId); - // - // before(async () => { - // await createTransformJob(transformId, destinationIndex); - // await transform.api.createIndices(destinationIndex); - // }); - // - // after(async () => { - // await transform.api.deleteIndices(destinationIndex); - // }); - // - // it('should delete transform and destination index', async () => { - // const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; - // const { body } = await supertest - // .post(`/api/transform/delete_transforms`) - // .auth( - // USER.TRANSFORM_POWERUSER, - // transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) - // ) - // .set(COMMON_REQUEST_HEADERS) - // .send({ - // transformsInfo, - // deleteDestIndex: true, - // }) - // .expect(200); - // - // expect(body[transformId].transformJobDeleted.success).to.eql(true); - // expect(body[transformId].destIndexDeleted.success).to.eql(true); - // expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); - // await transform.api.waitForTransformJobNotToExist(destinationIndex); - // await transform.api.waitForIndicesNotToExist(destinationIndex); - // }); - // it('should return 404 for unauthorized user', async () => { - // const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; - // const { body } = await supertest - // .post(`/api/transform/delete_transforms`) - // .auth( - // USER.TRANSFORM_VIEWER, - // transform.securityCommon.getPasswordForUser(USER.TRANSFORM_VIEWER) - // ) - // .set(COMMON_REQUEST_HEADERS) - // .send({ - // transformsInfo, - // deleteDestIndex: true, - // }) - // .expect(200); - // - // console.log('BODY', body); - // - // // expect(body.error).to.eql('Not Found'); - // // expect(body.message).to.eql('Not Found'); - // // await transform.api.waitForTransformJobToExist(destinationIndex); - // // await transform.api.waitForIndicesToExist(destinationIndex); - // }); - // }); }); }; diff --git a/x-pack/test/functional/services/transform/api.ts b/x-pack/test/functional/services/transform/api.ts index fc56559bc34f0..934b686aac4fd 100644 --- a/x-pack/test/functional/services/transform/api.ts +++ b/x-pack/test/functional/services/transform/api.ts @@ -12,12 +12,15 @@ import { TransformPivotConfig, TransformStats, } from '../../../../plugins/transform/public/app/common'; +import { COMMON_REQUEST_HEADERS } from '../ml/common'; +import { SavedObjectType } from '../ml/test_resources'; export function TransformAPIProvider({ getService }: FtrProviderContext) { const es = getService('legacyEs'); const log = getService('log'); const retry = getService('retry'); const esSupertest = getService('esSupertest'); + const supertest = getService('supertest'); return { async createIndices(indices: string) { @@ -32,7 +35,7 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { .to.have.property('acknowledged') .eql(true, 'Response for create request indices should be acknowledged.'); - await this.waitForIndicesToExist(indices); + await this.waitForIndicesToExist(indices, `expected ${indices} to be created`); }, async deleteIndices(indices: string) { @@ -52,12 +55,12 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { await this.waitForIndicesNotToExist(indices, `expected indices '${indices}' to be deleted`); }, - async waitForIndicesToExist(indices: string) { + async waitForIndicesToExist(indices: string, errorMsg?: string) { await retry.tryForTime(30 * 1000, async () => { if (await es.indices.exists({ index: indices, allowNoIndices: false })) { return true; } else { - throw new Error(`indices '${indices}' should exist`); + throw new Error(errorMsg || `indices '${indices}' should exist`); } }); }, @@ -76,6 +79,66 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { await this.deleteIndices('.transform-*'); }, + async createIndexPattern(title: string, timeFieldName?: string): Promise { + log.debug( + `Creating index pattern with title '${title}'${ + timeFieldName !== undefined ? ` and time field '${timeFieldName}'` : '' + }` + ); + + const createResponse = await supertest + .post(`/api/saved_objects/${SavedObjectType.INDEX_PATTERN}`) + .set(COMMON_REQUEST_HEADERS) + .send({ attributes: { title, timeFieldName } }) + .expect(200) + .then((res: any) => res.body); + + log.debug(` > Created with id '${createResponse.id}'`); + return createResponse.id; + }, + + async getSavedObjectIdByTitle( + title: string, + objectType: SavedObjectType + ): Promise { + log.debug(`Searching for '${objectType}' with title '${title}'...`); + const findResponse = await supertest + .get(`/api/saved_objects/_find?type=${objectType}`) + .set(COMMON_REQUEST_HEADERS) + .expect(200) + .then((res: any) => res.body); + + for (const savedObject of findResponse.saved_objects) { + const objectTitle = savedObject.attributes.title; + if (objectTitle === title) { + log.debug(` > Found '${savedObject.id}'`); + return savedObject.id; + } + } + log.debug(` > Not found`); + }, + + async getIndexPatternId(title: string): Promise { + return this.getSavedObjectIdByTitle(title, SavedObjectType.INDEX_PATTERN); + }, + + async deleteIndexPattern(title: string) { + log.debug(`Deleting index pattern with title '${title}'...`); + + const indexPatternId = await this.getIndexPatternId(title); + if (indexPatternId === undefined) { + log.debug(`Index pattern with title '${title}' does not exists. Nothing to delete.`); + return; + } else { + await supertest + .delete(`/api/saved_objects/${SavedObjectType.INDEX_PATTERN}/${indexPatternId}`) + .set(COMMON_REQUEST_HEADERS) + .expect(200); + + log.debug(` > Deleted index pattern with id '${indexPatternId}'`); + } + }, + async getTransformStats(transformId: string): Promise { log.debug(`Fetching transform stats for transform ${transformId}`); const statsResponse = await esSupertest @@ -137,6 +200,32 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { `expected transform '${transformId}' to be created` ); }, + + async createIndexPatternIfNeeded(title: string, timeFieldName?: string): Promise { + const indexPatternId = await this.getIndexPatternId(title); + if (indexPatternId !== undefined) { + log.debug(`Index pattern with title '${title}' already exists. Nothing to create.`); + return indexPatternId; + } else { + return await this.createIndexPattern(title, timeFieldName); + } + }, + + async waitForIndexPatternNotToExist(title: string) { + await retry.waitForWithTimeout( + `index pattern '${title}' to not exist`, + 5 * 1000, + async () => { + const indexPatternId = await this.getIndexPatternId(title); + if (!indexPatternId) { + return true; + } else { + throw new Error(`Index pattern '${title}' should not exist.`); + } + } + ); + }, + async waitForTransformJobToExist(transformId: string, errorMsg?: string) { await retry.waitForWithTimeout(`'${transformId}' to exist`, 5 * 1000, async () => { if (await this.getTransform(transformId, 200)) { diff --git a/x-pack/test/functional/services/transform/security_common.ts b/x-pack/test/functional/services/transform/security_common.ts index d4be4444a3c12..60347f154b924 100644 --- a/x-pack/test/functional/services/transform/security_common.ts +++ b/x-pack/test/functional/services/transform/security_common.ts @@ -21,7 +21,7 @@ export function TransformSecurityCommonProvider({ getService }: FtrProviderConte { name: 'transform_source', elasticsearch: { - indices: [{ names: ['*'], privileges: ['read', 'view_index_metadata'] }], + indices: [{ names: ['*'], privileges: ['read', 'view_index_metadata', 'manage'] }], }, kibana: [], }, From 90a2d5ec56b100a0294f1b27ae63f9ac934fbce3 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Wed, 3 Jun 2020 21:18:56 -0500 Subject: [PATCH 06/17] [Transform] Update bulk delete messages --- .../components/transform_list/action_delete.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx index 50f46db8ee705..20724fabfba9c 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx @@ -103,7 +103,7 @@ export const DeleteAction: FC = ({ items, forceDisable }) => label={i18n.translate( 'xpack.transform.actionDeleteTransform.bulkDeleteDestinationIndexTitle', { - defaultMessage: 'Delete destination index', + defaultMessage: 'Delete destination indexes', } )} checked={deleteDestIndex} @@ -118,7 +118,7 @@ export const DeleteAction: FC = ({ items, forceDisable }) => label={i18n.translate( 'xpack.transform.actionDeleteTransform.bulkDeleteDestIndexPatternTitle', { - defaultMessage: 'Delete index pattern', + defaultMessage: 'Delete destination index patterns', } )} checked={deleteIndexPattern} From d87248d5d50f0ecfccf212fa9d7b24a4d6aea40d Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Thu, 4 Jun 2020 08:22:58 -0500 Subject: [PATCH 07/17] [Transform] Update tests --- .../apis/transform/delete_transforms.ts | 87 ++++++++++++----- .../test/functional/services/transform/api.ts | 96 +------------------ .../services/transform/security_common.ts | 2 +- 3 files changed, 70 insertions(+), 115 deletions(-) diff --git a/x-pack/test/api_integration/apis/transform/delete_transforms.ts b/x-pack/test/api_integration/apis/transform/delete_transforms.ts index c282591621514..56e6cf66a09a1 100644 --- a/x-pack/test/api_integration/apis/transform/delete_transforms.ts +++ b/x-pack/test/api_integration/apis/transform/delete_transforms.ts @@ -9,6 +9,12 @@ import { FtrProviderContext } from '../../ftr_provider_context'; import { COMMON_REQUEST_HEADERS } from '../../../functional/services/ml/common'; import { USER } from '../../../functional/services/transform/security_common'; +async function asyncForEach(array: any[], callback: Function) { + for (let index = 0; index < array.length; index++) { + await callback(array[index], index, array); + } +} + // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext) => { const esArchiver = getService('esArchiver'); @@ -16,7 +22,7 @@ export default ({ getService }: FtrProviderContext) => { const transform = getService('transform'); function generateDestIndex(transformId: string): string { - return `dest_${transformId}`; + return `user-${transformId}`; } async function createTransformJob(transformId: string, destinationIndex: string) { @@ -73,7 +79,6 @@ export default ({ getService }: FtrProviderContext) => { expect(body[transformId].transformJobDeleted.success).to.eql(true); expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); - await transform.api.waitForIndicesToExist(destinationIndex); await transform.api.waitForTransformJobNotToExist(transformId); }); @@ -90,7 +95,6 @@ export default ({ getService }: FtrProviderContext) => { transformsInfo, }) .expect(403); - await transform.api.waitForIndicesToExist(destinationIndex); await transform.api.waitForTransformJobToExist(transformId); }); }); @@ -118,17 +122,19 @@ export default ({ getService }: FtrProviderContext) => { { id: 'bulk_delete_test_1' }, { id: 'bulk_delete_test_2' }, ]; + const destinationIndices = transformsInfo.map((d) => generateDestIndex(d.id)); beforeEach(async () => { - await createTransformJob('bulk_delete_test_1', 'bulk_delete_test_1'); - await transform.api.createIndices('bulk_delete_test_1'); - await createTransformJob('bulk_delete_test_2', 'bulk_delete_test_2'); - await transform.api.createIndices('bulk_delete_test_2'); + await asyncForEach(transformsInfo, async ({ id }: { id: string }, idx: number) => { + await createTransformJob(id, destinationIndices[idx]); + await transform.api.createIndices(destinationIndices[idx]); + }); }); afterEach(async () => { - await transform.api.deleteIndices('bulk_delete_test_1'); - await transform.api.deleteIndices('bulk_delete_test_2'); + await asyncForEach(destinationIndices, async (destinationIndex: string) => { + await transform.api.deleteIndices(destinationIndex); + }); }); it('should delete multiple transform jobs by transformIds', async () => { @@ -144,13 +150,12 @@ export default ({ getService }: FtrProviderContext) => { }) .expect(200); - transformsInfo.forEach(({ id: transformId }) => { + await asyncForEach(transformsInfo, async ({ id: transformId }: { id: string }) => { expect(body[transformId].transformJobDeleted.success).to.eql(true); expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); + await transform.api.waitForTransformJobNotToExist(transformId); }); - await transform.api.waitForTransformJobNotToExist('bulk_delete_test_1'); - await transform.api.waitForTransformJobNotToExist('bulk_delete_test_2'); }); it('should delete multiple transform jobs by transformIds, even if one of the transformIds is invalid', async () => { @@ -164,20 +169,20 @@ export default ({ getService }: FtrProviderContext) => { .set(COMMON_REQUEST_HEADERS) .send({ transformsInfo: [ - { id: 'bulk_delete_test_1' }, + { id: transformsInfo[0].id }, { id: invalidTransformId }, - { id: 'bulk_delete_test_2' }, + { id: transformsInfo[1].id }, ], }) .expect(200); - transformsInfo.forEach(({ id: transformId }) => { + await asyncForEach(transformsInfo, async ({ id: transformId }: { id: string }) => { expect(body[transformId].transformJobDeleted.success).to.eql(true); expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); + await transform.api.waitForTransformJobNotToExist(transformId); }); - await transform.api.waitForTransformJobNotToExist('bulk_delete_test_1'); - await transform.api.waitForTransformJobNotToExist('bulk_delete_test_2'); + expect(body[invalidTransformId].transformJobDeleted.success).to.eql(false); expect(body[invalidTransformId].transformJobDeleted).to.have.property('error'); }); @@ -226,13 +231,12 @@ export default ({ getService }: FtrProviderContext) => { before(async () => { await createTransformJob(transformId, destinationIndex); await transform.api.createIndices(destinationIndex); - await transform.api.waitForIndicesToExist(destinationIndex); - await transform.api.createIndexPatternIfNeeded(destinationIndex); + await transform.testResources.createIndexPatternIfNeeded(destinationIndex); }); after(async () => { await transform.api.deleteIndices(destinationIndex); - await transform.api.deleteIndexPattern(destinationIndex); + await transform.testResources.deleteIndexPattern(destinationIndex); }); it('should delete transform and destination index pattern', async () => { @@ -255,8 +259,47 @@ export default ({ getService }: FtrProviderContext) => { expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(true); await transform.api.waitForTransformJobNotToExist(transformId); - await transform.api.waitForIndicesToExist(destinationIndex); - await transform.api.waitForIndexPatternNotToExist(destinationIndex); + await transform.testResources.assertIndexPatternNotExist(destinationIndex); + }); + }); + + describe('with deleteDestIndex & deleteDestIndexPattern setting', function () { + const transformId = 'test4'; + const destinationIndex = generateDestIndex(transformId); + + before(async () => { + await createTransformJob(transformId, destinationIndex); + await transform.api.createIndices(destinationIndex); + await transform.testResources.createIndexPatternIfNeeded(destinationIndex); + }); + + after(async () => { + await transform.api.deleteIndices(destinationIndex); + await transform.testResources.deleteIndexPattern(destinationIndex); + }); + + it('should delete transform, destination index, & destination index pattern', async () => { + const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; + const { body } = await supertest + .post(`/api/transform/delete_transforms`) + .auth( + USER.TRANSFORM_POWERUSER, + transform.securityCommon.getPasswordForUser(USER.TRANSFORM_POWERUSER) + ) + .set(COMMON_REQUEST_HEADERS) + .send({ + transformsInfo, + deleteDestIndex: true, + deleteDestIndexPattern: true, + }) + .expect(200); + + expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].destIndexDeleted.success).to.eql(true); + expect(body[transformId].destIndexPatternDeleted.success).to.eql(true); + await transform.api.waitForTransformJobNotToExist(transformId); + await transform.api.waitForIndicesNotToExist(destinationIndex); + await transform.testResources.assertIndexPatternNotExist(destinationIndex); }); }); }); diff --git a/x-pack/test/functional/services/transform/api.ts b/x-pack/test/functional/services/transform/api.ts index 934b686aac4fd..2895bf2bc719e 100644 --- a/x-pack/test/functional/services/transform/api.ts +++ b/x-pack/test/functional/services/transform/api.ts @@ -12,20 +12,17 @@ import { TransformPivotConfig, TransformStats, } from '../../../../plugins/transform/public/app/common'; -import { COMMON_REQUEST_HEADERS } from '../ml/common'; -import { SavedObjectType } from '../ml/test_resources'; export function TransformAPIProvider({ getService }: FtrProviderContext) { const es = getService('legacyEs'); const log = getService('log'); const retry = getService('retry'); const esSupertest = getService('esSupertest'); - const supertest = getService('supertest'); return { async createIndices(indices: string) { log.debug(`Creating indices: '${indices}'...`); - if (await es.indices.exists({ index: indices, allowNoIndices: false })) { + if ((await es.indices.exists({ index: indices, allowNoIndices: false })) === true) { log.debug(`Indices '${indices}' already exist. Nothing to create.`); return; } @@ -40,7 +37,7 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { async deleteIndices(indices: string) { log.debug(`Deleting indices: '${indices}'...`); - if (!(await es.indices.exists({ index: indices, allowNoIndices: false }))) { + if ((await es.indices.exists({ index: indices, allowNoIndices: false })) === false) { log.debug(`Indices '${indices}' don't exist. Nothing to delete.`); return; } @@ -57,7 +54,7 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { async waitForIndicesToExist(indices: string, errorMsg?: string) { await retry.tryForTime(30 * 1000, async () => { - if (await es.indices.exists({ index: indices, allowNoIndices: false })) { + if ((await es.indices.exists({ index: indices, allowNoIndices: false })) === true) { return true; } else { throw new Error(errorMsg || `indices '${indices}' should exist`); @@ -67,7 +64,7 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { async waitForIndicesNotToExist(indices: string, errorMsg?: string) { await retry.tryForTime(30 * 1000, async () => { - if (!(await es.indices.exists({ index: indices, allowNoIndices: false }))) { + if ((await es.indices.exists({ index: indices, allowNoIndices: false })) === false) { return true; } else { throw new Error(errorMsg || `indices '${indices}' should not exist`); @@ -79,66 +76,6 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { await this.deleteIndices('.transform-*'); }, - async createIndexPattern(title: string, timeFieldName?: string): Promise { - log.debug( - `Creating index pattern with title '${title}'${ - timeFieldName !== undefined ? ` and time field '${timeFieldName}'` : '' - }` - ); - - const createResponse = await supertest - .post(`/api/saved_objects/${SavedObjectType.INDEX_PATTERN}`) - .set(COMMON_REQUEST_HEADERS) - .send({ attributes: { title, timeFieldName } }) - .expect(200) - .then((res: any) => res.body); - - log.debug(` > Created with id '${createResponse.id}'`); - return createResponse.id; - }, - - async getSavedObjectIdByTitle( - title: string, - objectType: SavedObjectType - ): Promise { - log.debug(`Searching for '${objectType}' with title '${title}'...`); - const findResponse = await supertest - .get(`/api/saved_objects/_find?type=${objectType}`) - .set(COMMON_REQUEST_HEADERS) - .expect(200) - .then((res: any) => res.body); - - for (const savedObject of findResponse.saved_objects) { - const objectTitle = savedObject.attributes.title; - if (objectTitle === title) { - log.debug(` > Found '${savedObject.id}'`); - return savedObject.id; - } - } - log.debug(` > Not found`); - }, - - async getIndexPatternId(title: string): Promise { - return this.getSavedObjectIdByTitle(title, SavedObjectType.INDEX_PATTERN); - }, - - async deleteIndexPattern(title: string) { - log.debug(`Deleting index pattern with title '${title}'...`); - - const indexPatternId = await this.getIndexPatternId(title); - if (indexPatternId === undefined) { - log.debug(`Index pattern with title '${title}' does not exists. Nothing to delete.`); - return; - } else { - await supertest - .delete(`/api/saved_objects/${SavedObjectType.INDEX_PATTERN}/${indexPatternId}`) - .set(COMMON_REQUEST_HEADERS) - .expect(200); - - log.debug(` > Deleted index pattern with id '${indexPatternId}'`); - } - }, - async getTransformStats(transformId: string): Promise { log.debug(`Fetching transform stats for transform ${transformId}`); const statsResponse = await esSupertest @@ -201,31 +138,6 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { ); }, - async createIndexPatternIfNeeded(title: string, timeFieldName?: string): Promise { - const indexPatternId = await this.getIndexPatternId(title); - if (indexPatternId !== undefined) { - log.debug(`Index pattern with title '${title}' already exists. Nothing to create.`); - return indexPatternId; - } else { - return await this.createIndexPattern(title, timeFieldName); - } - }, - - async waitForIndexPatternNotToExist(title: string) { - await retry.waitForWithTimeout( - `index pattern '${title}' to not exist`, - 5 * 1000, - async () => { - const indexPatternId = await this.getIndexPatternId(title); - if (!indexPatternId) { - return true; - } else { - throw new Error(`Index pattern '${title}' should not exist.`); - } - } - ); - }, - async waitForTransformJobToExist(transformId: string, errorMsg?: string) { await retry.waitForWithTimeout(`'${transformId}' to exist`, 5 * 1000, async () => { if (await this.getTransform(transformId, 200)) { diff --git a/x-pack/test/functional/services/transform/security_common.ts b/x-pack/test/functional/services/transform/security_common.ts index 60347f154b924..d4be4444a3c12 100644 --- a/x-pack/test/functional/services/transform/security_common.ts +++ b/x-pack/test/functional/services/transform/security_common.ts @@ -21,7 +21,7 @@ export function TransformSecurityCommonProvider({ getService }: FtrProviderConte { name: 'transform_source', elasticsearch: { - indices: [{ names: ['*'], privileges: ['read', 'view_index_metadata', 'manage'] }], + indices: [{ names: ['*'], privileges: ['read', 'view_index_metadata'] }], }, kibana: [], }, From d3c37b5c0d03c2ca6e211a3ff2380154223c2f0d Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Thu, 4 Jun 2020 09:37:41 -0500 Subject: [PATCH 08/17] [Transform] Rename transform job -> transform, fix ts --- x-pack/plugins/transform/common/index.ts | 4 +- .../public/app/hooks/use_delete_transform.tsx | 45 +++++++++-------- .../transform_list/action_delete.tsx | 18 +++---- .../transform/server/routes/api/transforms.ts | 22 ++++---- .../apis/transform/delete_transforms.ts | 50 +++++++++---------- .../test/functional/services/transform/api.ts | 6 +-- 6 files changed, 75 insertions(+), 70 deletions(-) diff --git a/x-pack/plugins/transform/common/index.ts b/x-pack/plugins/transform/common/index.ts index 3fb2c2071c483..79ff6298a2ca2 100644 --- a/x-pack/plugins/transform/common/index.ts +++ b/x-pack/plugins/transform/common/index.ts @@ -39,14 +39,14 @@ export interface TransformEndpointResult { [key: string]: ResultData; } -export interface DeleteTransformEndpoint { +export interface DeleteTransformEndpointRequest { transformsInfo: TransformEndpointRequest[]; deleteDestIndex?: boolean; deleteDestIndexPattern?: boolean; } export interface DeleteTransformStatus { - transformJobDeleted: ResultData; + transformDeleted: ResultData; destIndexDeleted?: ResultData; destIndexPatternDeleted?: ResultData; destinationIndex?: string | undefined; diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index e553b6c00eed0..3999ca022251e 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -8,7 +8,11 @@ import React, { useCallback, useEffect, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { CoreSetup } from 'kibana/public'; import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public'; -import { TransformEndpointRequest, DeleteTransformEndpointResult } from '../../../common'; +import { + TransformEndpointRequest, + DeleteTransformEndpointResult, + DeleteTransformStatus, +} from '../../../common'; import { getErrorMessage, extractErrorMessage } from '../../shared_imports'; import { useAppDependencies, useToastNotifications } from '../app_dependencies'; import { TransformListRow, refreshTransformList$, REFRESH_TRANSFORM_LIST_STATE } from '../common'; @@ -62,7 +66,7 @@ export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { toastNotifications.addDanger( i18n.translate( - 'xpack.transform.deleteTransfrom.errorWithCheckingIfIndexPatternExistsNotificationErrorMessage', + 'xpack.transform.deleteTransform.errorWithCheckingIfIndexPatternExistsNotificationErrorMessage', { defaultMessage: 'An error occurred checking if index pattern {indexPattern} exists: {error}', @@ -117,6 +121,8 @@ export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { }; }; +type SuccessCountField = keyof Omit; + export const useDeleteTransforms = () => { const { overlays } = useAppDependencies(); const toastNotifications = useToastNotifications(); @@ -139,8 +145,8 @@ export const useDeleteTransforms = () => { shouldDeleteDestIndexPattern ); const isBulk = Object.keys(results).length > 1; - const successCount: Record = { - transformJobDeleted: 0, + const successCount: Record = { + transformDeleted: 0, destIndexDeleted: 0, destIndexPatternDeleted: 0, }; @@ -150,12 +156,12 @@ export const useDeleteTransforms = () => { const status = results[transformId]; const destinationIndex = status.destinationIndex; - // if we are only deleting one modal, show the success toast messages + // if we are only deleting one transform, show the success toast messages if (!isBulk) { - if (status.transformJobDeleted?.success) { + if (status.transformDeleted?.success) { toastNotifications.addSuccess( i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { - defaultMessage: 'Request to delete transform job {transformId} acknowledged.', + defaultMessage: 'Request to delete transform {transformId} acknowledged.', values: { transformId }, }) ); @@ -163,7 +169,7 @@ export const useDeleteTransforms = () => { if (status.destIndexDeleted?.success) { toastNotifications.addSuccess( i18n.translate( - 'xpack.transform.deleteTransfrom.deleteAnalyticsWithIndexSuccessMessage', + 'xpack.transform.deleteTransform.deleteAnalyticsWithIndexSuccessMessage', { defaultMessage: 'Request to delete destination index {destinationIndex} acknowledged.', @@ -175,7 +181,7 @@ export const useDeleteTransforms = () => { if (status.destIndexPatternDeleted?.success) { toastNotifications.addSuccess( i18n.translate( - 'xpack.transform.deleteTransfrom.deleteAnalyticsWithIndexPatternSuccessMessage', + 'xpack.transform.deleteTransform.deleteAnalyticsWithIndexPatternSuccessMessage', { defaultMessage: 'Request to delete index pattern {destinationIndex} acknowledged.', @@ -185,19 +191,17 @@ export const useDeleteTransforms = () => { ); } } else { - Object.keys(successCount).forEach((key) => { - // @ts-ignore + (Object.keys(successCount) as SuccessCountField[]).forEach((key) => { if (status[key]?.success) { successCount[key] = successCount[key] + 1; } }); } - if (status.transformJobDeleted?.error) { - const error = extractErrorMessage(status.transformJobDeleted.error); + if (status.transformDeleted?.error) { + const error = extractErrorMessage(status.transformDeleted.error); toastNotifications.addDanger({ title: i18n.translate('xpack.transform.transformList.deleteTransformErrorMessage', { - defaultMessage: - 'An error occurred deleting the data frame analytics job {transformId}', + defaultMessage: 'An error occurred deleting the transform {transformId}', values: { transformId }, }), text: toMountPoint( @@ -210,7 +214,7 @@ export const useDeleteTransforms = () => { const error = extractErrorMessage(status.destIndexDeleted.error); toastNotifications.addDanger({ title: i18n.translate( - 'xpack.transform.deleteTransfrom.deleteAnalyticsWithIndexErrorMessage', + 'xpack.transform.deleteTransform.deleteAnalyticsWithIndexErrorMessage', { defaultMessage: 'An error occurred deleting destination index {destinationIndex}', values: { destinationIndex }, @@ -226,7 +230,7 @@ export const useDeleteTransforms = () => { const error = extractErrorMessage(status.destIndexPatternDeleted.error); toastNotifications.addDanger({ title: i18n.translate( - 'xpack.transform.deleteTransfrom.deleteAnalyticsWithIndexPatternErrorMessage', + 'xpack.transform.deleteTransform.deleteAnalyticsWithIndexPatternErrorMessage', { defaultMessage: 'An error occurred deleting index pattern {destinationIndex}', values: { destinationIndex }, @@ -240,13 +244,14 @@ export const useDeleteTransforms = () => { } } + // if we are deleting multiple transforms, combine the success messages if (isBulk) { - if (successCount.transformJobDeleted > 0) { + if (successCount.transformDeleted > 0) { toastNotifications.addSuccess( i18n.translate('xpack.transform.transformList.bulkDeleteTransformSuccessMessage', { defaultMessage: - 'Successfully deleted {count} transform {count, plural, one {job} other {jobs}}.', - values: { count: successCount.transformJobDeleted }, + 'Successfully deleted {count} {count, plural, one {transform} other {transforms}}.', + values: { count: successCount.transformDeleted }, }) ); } diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx index 20724fabfba9c..03dcb575f477f 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx @@ -85,7 +85,7 @@ export const DeleteAction: FC = ({ items, forceDisable }) => defaultMessage: 'Delete {transformId}', values: { transformId: items[0] && items[0].config.id }, }); - const bulkDeleteModalMessage = ( + const bulkDeleteModalContent = ( <>

= ({ items, forceDisable }) => { = ({ items, forceDisable }) => { = ({ items, forceDisable }) => ); - const deleteModalMessage = ( - + const deleteModalContent = ( + <>

= ({ items, forceDisable }) => {userCanDeleteIndex && ( = ({ items, forceDisable }) => {userCanDeleteIndex && indexPatternExists && ( = ({ items, forceDisable }) => )} - + ); let deleteButton = ( @@ -228,7 +228,7 @@ export const DeleteAction: FC = ({ items, forceDisable }) => defaultFocusedButton={EUI_MODAL_CONFIRM_BUTTON} buttonColor="danger" > - {isBulkAction ? bulkDeleteModalMessage : deleteModalMessage} + {isBulkAction ? bulkDeleteModalContent : deleteModalContent} )} diff --git a/x-pack/plugins/transform/server/routes/api/transforms.ts b/x-pack/plugins/transform/server/routes/api/transforms.ts index 50cf9fa38fcfe..6c1d288c18148 100644 --- a/x-pack/plugins/transform/server/routes/api/transforms.ts +++ b/x-pack/plugins/transform/server/routes/api/transforms.ts @@ -19,7 +19,7 @@ import { TransformEndpointResult, TransformId, TRANSFORM_STATE, - DeleteTransformEndpoint, + DeleteTransformEndpointRequest, DeleteTransformStatus, ResultData, } from '../../../common'; @@ -185,7 +185,7 @@ export function registerTransformsRoutes(routeDependencies: RouteDependencies) { body: schema.maybe(schema.any()), query: schema.object({ /** - * Analytics Destination Index + * Transform Destination Index */ deleteDestIndex: schema.maybe(schema.boolean()), deleteDestIndexPattern: schema.maybe(schema.boolean()), @@ -197,7 +197,7 @@ export function registerTransformsRoutes(routeDependencies: RouteDependencies) { transformsInfo, deleteDestIndex, deleteDestIndexPattern, - } = req.body as DeleteTransformEndpoint; + } = req.body as DeleteTransformEndpointRequest; try { const body = await deleteTransforms( @@ -315,7 +315,7 @@ async function deleteTransforms( for (const transformInfo of transformsInfo) { let destinationIndex: string | undefined; - const transformJobDeleted: ResultData = { success: false }; + const transformDeleted: ResultData = { success: false }; const destIndexDeleted: ResultData = { success: false }; const destIndexPatternDeleted: ResultData = { success: false, @@ -351,9 +351,9 @@ async function deleteTransforms( ? transformConfig.dest.index[0] : transformConfig.dest.index; } catch (getTransformConfigError) { - transformJobDeleted.error = wrapError(getTransformConfigError); + transformDeleted.error = wrapError(getTransformConfigError); results[transformId] = { - transformJobDeleted, + transformDeleted, destIndexDeleted, destIndexPatternDeleted, destinationIndex, @@ -395,16 +395,16 @@ async function deleteTransforms( await ctx.transform!.dataClient.callAsCurrentUser('transform.deleteTransform', { transformId, }); - transformJobDeleted.success = true; + transformDeleted.success = true; } catch (deleteTransformJobError) { - transformJobDeleted.error = wrapError(deleteTransformJobError); - if (transformJobDeleted.error.statusCode === 403) { + transformDeleted.error = wrapError(deleteTransformJobError); + if (transformDeleted.error.statusCode === 403) { return response.forbidden(); } } results[transformId] = { - transformJobDeleted, + transformDeleted, destIndexDeleted, destIndexPatternDeleted, destinationIndex, @@ -418,7 +418,7 @@ async function deleteTransforms( action: TRANSFORM_ACTIONS.DELETE, }); } - results[transformId] = { transformJobDeleted: { success: false, error: JSON.stringify(e) } }; + results[transformId] = { transformDeleted: { success: false, error: JSON.stringify(e) } }; } } return results; diff --git a/x-pack/test/api_integration/apis/transform/delete_transforms.ts b/x-pack/test/api_integration/apis/transform/delete_transforms.ts index 56e6cf66a09a1..84051234364c6 100644 --- a/x-pack/test/api_integration/apis/transform/delete_transforms.ts +++ b/x-pack/test/api_integration/apis/transform/delete_transforms.ts @@ -25,7 +25,7 @@ export default ({ getService }: FtrProviderContext) => { return `user-${transformId}`; } - async function createTransformJob(transformId: string, destinationIndex: string) { + async function createTransform(transformId: string, destinationIndex: string) { const config = { id: transformId, source: { index: ['farequote-*'] }, @@ -54,7 +54,7 @@ export default ({ getService }: FtrProviderContext) => { const destinationIndex = generateDestIndex(transformId); beforeEach(async () => { - await createTransformJob(transformId, destinationIndex); + await createTransform(transformId, destinationIndex); await transform.api.createIndices(destinationIndex); }); @@ -62,7 +62,7 @@ export default ({ getService }: FtrProviderContext) => { await transform.api.deleteIndices(destinationIndex); }); - it('should delete transform job by transformId', async () => { + it('should delete transform by transformId', async () => { const transformsInfo: TransformEndpointRequest[] = [{ id: transformId }]; const { body } = await supertest .post(`/api/transform/delete_transforms`) @@ -76,10 +76,10 @@ export default ({ getService }: FtrProviderContext) => { }) .expect(200); - expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].transformDeleted.success).to.eql(true); expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); - await transform.api.waitForTransformJobNotToExist(transformId); + await transform.api.waitForTransformNotToExist(transformId); }); it('should return 403 for unauthorized user', async () => { @@ -95,7 +95,7 @@ export default ({ getService }: FtrProviderContext) => { transformsInfo, }) .expect(403); - await transform.api.waitForTransformJobToExist(transformId); + await transform.api.waitForTransformToExist(transformId); }); }); @@ -113,7 +113,7 @@ export default ({ getService }: FtrProviderContext) => { transformsInfo, }) .expect(200); - expect(body.invalid_transform_id.transformJobDeleted.success).to.eql(false); + expect(body.invalid_transform_id.transformDeleted.success).to.eql(false); }); }); @@ -126,7 +126,7 @@ export default ({ getService }: FtrProviderContext) => { beforeEach(async () => { await asyncForEach(transformsInfo, async ({ id }: { id: string }, idx: number) => { - await createTransformJob(id, destinationIndices[idx]); + await createTransform(id, destinationIndices[idx]); await transform.api.createIndices(destinationIndices[idx]); }); }); @@ -137,7 +137,7 @@ export default ({ getService }: FtrProviderContext) => { }); }); - it('should delete multiple transform jobs by transformIds', async () => { + it('should delete multiple transforms by transformIds', async () => { const { body } = await supertest .post(`/api/transform/delete_transforms`) .auth( @@ -151,14 +151,14 @@ export default ({ getService }: FtrProviderContext) => { .expect(200); await asyncForEach(transformsInfo, async ({ id: transformId }: { id: string }) => { - expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].transformDeleted.success).to.eql(true); expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); - await transform.api.waitForTransformJobNotToExist(transformId); + await transform.api.waitForTransformNotToExist(transformId); }); }); - it('should delete multiple transform jobs by transformIds, even if one of the transformIds is invalid', async () => { + it('should delete multiple transforms by transformIds, even if one of the transformIds is invalid', async () => { const invalidTransformId = 'invalid_transform_id'; const { body } = await supertest .post(`/api/transform/delete_transforms`) @@ -177,14 +177,14 @@ export default ({ getService }: FtrProviderContext) => { .expect(200); await asyncForEach(transformsInfo, async ({ id: transformId }: { id: string }) => { - expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].transformDeleted.success).to.eql(true); expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); - await transform.api.waitForTransformJobNotToExist(transformId); + await transform.api.waitForTransformNotToExist(transformId); }); - expect(body[invalidTransformId].transformJobDeleted.success).to.eql(false); - expect(body[invalidTransformId].transformJobDeleted).to.have.property('error'); + expect(body[invalidTransformId].transformDeleted.success).to.eql(false); + expect(body[invalidTransformId].transformDeleted).to.have.property('error'); }); }); @@ -193,7 +193,7 @@ export default ({ getService }: FtrProviderContext) => { const destinationIndex = generateDestIndex(transformId); before(async () => { - await createTransformJob(transformId, destinationIndex); + await createTransform(transformId, destinationIndex); await transform.api.createIndices(destinationIndex); }); @@ -216,10 +216,10 @@ export default ({ getService }: FtrProviderContext) => { }) .expect(200); - expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].transformDeleted.success).to.eql(true); expect(body[transformId].destIndexDeleted.success).to.eql(true); expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); - await transform.api.waitForTransformJobNotToExist(transformId); + await transform.api.waitForTransformNotToExist(transformId); await transform.api.waitForIndicesNotToExist(destinationIndex); }); }); @@ -229,7 +229,7 @@ export default ({ getService }: FtrProviderContext) => { const destinationIndex = generateDestIndex(transformId); before(async () => { - await createTransformJob(transformId, destinationIndex); + await createTransform(transformId, destinationIndex); await transform.api.createIndices(destinationIndex); await transform.testResources.createIndexPatternIfNeeded(destinationIndex); }); @@ -255,10 +255,10 @@ export default ({ getService }: FtrProviderContext) => { }) .expect(200); - expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].transformDeleted.success).to.eql(true); expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(true); - await transform.api.waitForTransformJobNotToExist(transformId); + await transform.api.waitForTransformNotToExist(transformId); await transform.testResources.assertIndexPatternNotExist(destinationIndex); }); }); @@ -268,7 +268,7 @@ export default ({ getService }: FtrProviderContext) => { const destinationIndex = generateDestIndex(transformId); before(async () => { - await createTransformJob(transformId, destinationIndex); + await createTransform(transformId, destinationIndex); await transform.api.createIndices(destinationIndex); await transform.testResources.createIndexPatternIfNeeded(destinationIndex); }); @@ -294,10 +294,10 @@ export default ({ getService }: FtrProviderContext) => { }) .expect(200); - expect(body[transformId].transformJobDeleted.success).to.eql(true); + expect(body[transformId].transformDeleted.success).to.eql(true); expect(body[transformId].destIndexDeleted.success).to.eql(true); expect(body[transformId].destIndexPatternDeleted.success).to.eql(true); - await transform.api.waitForTransformJobNotToExist(transformId); + await transform.api.waitForTransformNotToExist(transformId); await transform.api.waitForIndicesNotToExist(destinationIndex); await transform.testResources.assertIndexPatternNotExist(destinationIndex); }); diff --git a/x-pack/test/functional/services/transform/api.ts b/x-pack/test/functional/services/transform/api.ts index 2895bf2bc719e..697020fafb196 100644 --- a/x-pack/test/functional/services/transform/api.ts +++ b/x-pack/test/functional/services/transform/api.ts @@ -132,13 +132,13 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { log.debug(`Creating transform with id '${transformId}'...`); await esSupertest.put(`/_transform/${transformId}`).send(transformConfig).expect(200); - await this.waitForTransformJobToExist( + await this.waitForTransformToExist( transformId, `expected transform '${transformId}' to be created` ); }, - async waitForTransformJobToExist(transformId: string, errorMsg?: string) { + async waitForTransformToExist(transformId: string, errorMsg?: string) { await retry.waitForWithTimeout(`'${transformId}' to exist`, 5 * 1000, async () => { if (await this.getTransform(transformId, 200)) { return true; @@ -147,7 +147,7 @@ export function TransformAPIProvider({ getService }: FtrProviderContext) { } }); }, - async waitForTransformJobNotToExist(transformId: string, errorMsg?: string) { + async waitForTransformNotToExist(transformId: string, errorMsg?: string) { await retry.waitForWithTimeout(`'${transformId}' to exist`, 5 * 1000, async () => { if (await this.getTransform(transformId, 404)) { return true; From ec3acd3ffcec0cdf17fa414a355208ca0e5ce9f8 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Thu, 4 Jun 2020 10:20:52 -0500 Subject: [PATCH 09/17] [Transform] Add await to userCanDelete callback --- .../transform/public/app/hooks/use_delete_transform.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index 3999ca022251e..f47dc885f5013 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -79,9 +79,9 @@ export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { [savedObjectsClient, toastNotifications] ); - const checkUserIndexPermission = useCallback(() => { + const checkUserIndexPermission = useCallback(async () => { try { - const userCanDelete = canDeleteIndex(http); + const userCanDelete = await canDeleteIndex(http); if (userCanDelete) { setUserCanDeleteIndex(true); } From 1269b1d3f026e13d9b854f6afba6f58aedf6f295 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Thu, 4 Jun 2020 11:32:48 -0500 Subject: [PATCH 10/17] [Transform] Remove .toLowerCase check in getIP --- x-pack/plugins/transform/server/routes/api/transforms.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/x-pack/plugins/transform/server/routes/api/transforms.ts b/x-pack/plugins/transform/server/routes/api/transforms.ts index 6c1d288c18148..9026069c712a0 100644 --- a/x-pack/plugins/transform/server/routes/api/transforms.ts +++ b/x-pack/plugins/transform/server/routes/api/transforms.ts @@ -287,11 +287,7 @@ async function getIndexPatternId( searchFields: ['title'], fields: ['title'], }); - - const ip = response.saved_objects.find( - (obj) => obj.attributes.title.toLowerCase() === indexName.toLowerCase() - ); - + const ip = response.saved_objects.find((obj) => obj.attributes.title === indexName); return ip?.id; } From d8f901c118e09f44b99fb4a8c5a2d50fda4365a4 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 5 Jun 2020 11:28:00 -0500 Subject: [PATCH 11/17] [Transform] Update tests to include waitForIndicesToExist --- .../apis/transform/delete_transforms.ts | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/x-pack/test/api_integration/apis/transform/delete_transforms.ts b/x-pack/test/api_integration/apis/transform/delete_transforms.ts index 84051234364c6..bde1af098013d 100644 --- a/x-pack/test/api_integration/apis/transform/delete_transforms.ts +++ b/x-pack/test/api_integration/apis/transform/delete_transforms.ts @@ -80,6 +80,7 @@ export default ({ getService }: FtrProviderContext) => { expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); await transform.api.waitForTransformNotToExist(transformId); + await transform.api.waitForIndicesToExist(destinationIndex); }); it('should return 403 for unauthorized user', async () => { @@ -96,11 +97,12 @@ export default ({ getService }: FtrProviderContext) => { }) .expect(403); await transform.api.waitForTransformToExist(transformId); + await transform.api.waitForIndicesToExist(destinationIndex); }); }); describe('single transform deletion with invalid transformId', function () { - it('should return 404 with error message if invalid transformId', async () => { + it('should return 200 with error in response if invalid transformId', async () => { const transformsInfo: TransformEndpointRequest[] = [{ id: 'invalid_transform_id' }]; const { body } = await supertest .post(`/api/transform/delete_transforms`) @@ -150,12 +152,16 @@ export default ({ getService }: FtrProviderContext) => { }) .expect(200); - await asyncForEach(transformsInfo, async ({ id: transformId }: { id: string }) => { - expect(body[transformId].transformDeleted.success).to.eql(true); - expect(body[transformId].destIndexDeleted.success).to.eql(false); - expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); - await transform.api.waitForTransformNotToExist(transformId); - }); + await asyncForEach( + transformsInfo, + async ({ id: transformId }: { id: string }, idx: number) => { + expect(body[transformId].transformDeleted.success).to.eql(true); + expect(body[transformId].destIndexDeleted.success).to.eql(false); + expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); + await transform.api.waitForTransformNotToExist(transformId); + await transform.api.waitForIndicesToExist(destinationIndices[idx]); + } + ); }); it('should delete multiple transforms by transformIds, even if one of the transformIds is invalid', async () => { @@ -176,12 +182,16 @@ export default ({ getService }: FtrProviderContext) => { }) .expect(200); - await asyncForEach(transformsInfo, async ({ id: transformId }: { id: string }) => { - expect(body[transformId].transformDeleted.success).to.eql(true); - expect(body[transformId].destIndexDeleted.success).to.eql(false); - expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); - await transform.api.waitForTransformNotToExist(transformId); - }); + await asyncForEach( + transformsInfo, + async ({ id: transformId }: { id: string }, idx: number) => { + expect(body[transformId].transformDeleted.success).to.eql(true); + expect(body[transformId].destIndexDeleted.success).to.eql(false); + expect(body[transformId].destIndexPatternDeleted.success).to.eql(false); + await transform.api.waitForTransformNotToExist(transformId); + await transform.api.waitForIndicesToExist(destinationIndices[idx]); + } + ); expect(body[invalidTransformId].transformDeleted.success).to.eql(false); expect(body[invalidTransformId].transformDeleted).to.have.property('error'); @@ -259,6 +269,7 @@ export default ({ getService }: FtrProviderContext) => { expect(body[transformId].destIndexDeleted.success).to.eql(false); expect(body[transformId].destIndexPatternDeleted.success).to.eql(true); await transform.api.waitForTransformNotToExist(transformId); + await transform.api.waitForIndicesToExist(destinationIndex); await transform.testResources.assertIndexPatternNotExist(destinationIndex); }); }); From cd293f22098558e0c2b17a256658f5622101d0e5 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 5 Jun 2020 11:30:10 -0500 Subject: [PATCH 12/17] [Transform] Refactor delete transform schema --- .../transform/server/routes/api/schema.ts | 16 ++++++++++++++++ .../transform/server/routes/api/transforms.ts | 11 ++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/transform/server/routes/api/schema.ts b/x-pack/plugins/transform/server/routes/api/schema.ts index 0b994406d324d..499147647b224 100644 --- a/x-pack/plugins/transform/server/routes/api/schema.ts +++ b/x-pack/plugins/transform/server/routes/api/schema.ts @@ -14,3 +14,19 @@ export const schemaTransformId = { export interface SchemaTransformId { transformId: string; } + +export const deleteTransformSchema = schema.object({ + /** + * Delete Transform & Destination Index + */ + transformsInfo: schema.maybe( + schema.arrayOf( + schema.object({ + id: schema.string(), + state: schema.maybe(schema.string()), + }) + ) + ), + deleteDestIndex: schema.maybe(schema.boolean()), + deleteDestIndexPattern: schema.maybe(schema.boolean()), +}); diff --git a/x-pack/plugins/transform/server/routes/api/transforms.ts b/x-pack/plugins/transform/server/routes/api/transforms.ts index 9026069c712a0..0557ccaa2a8d3 100644 --- a/x-pack/plugins/transform/server/routes/api/transforms.ts +++ b/x-pack/plugins/transform/server/routes/api/transforms.ts @@ -29,7 +29,7 @@ import { RouteDependencies } from '../../types'; import { addBasePath } from '../index'; import { isRequestTimeout, fillResultsWithTimeouts, wrapError } from './error_utils'; -import { schemaTransformId, SchemaTransformId } from './schema'; +import { deleteTransformSchema, schemaTransformId, SchemaTransformId } from './schema'; import { registerTransformsAuditMessagesRoutes } from './transforms_audit_messages'; import { IIndexPattern } from '../../../../../../src/plugins/data/common/index_patterns'; @@ -182,14 +182,7 @@ export function registerTransformsRoutes(routeDependencies: RouteDependencies) { { path: addBasePath('delete_transforms'), validate: { - body: schema.maybe(schema.any()), - query: schema.object({ - /** - * Transform Destination Index - */ - deleteDestIndex: schema.maybe(schema.boolean()), - deleteDestIndexPattern: schema.maybe(schema.boolean()), - }), + body: deleteTransformSchema, }, }, license.guardApiRoute(async (ctx, req, res) => { From 8e7b21a5299a30b92338c51689638dc8126a756a Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 5 Jun 2020 11:33:59 -0500 Subject: [PATCH 13/17] [Transform] Add spacer, rename indices, --- .../app/components/toast_notification_text.tsx | 2 +- .../public/app/hooks/use_delete_transform.tsx | 2 +- .../components/transform_list/action_delete.tsx | 15 ++++++++------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx b/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx index dcbbca2746d99..3f664cf8bb09b 100644 --- a/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx +++ b/x-pack/plugins/transform/public/app/components/toast_notification_text.tsx @@ -51,7 +51,7 @@ export const ToastNotificationText: FC = ({ const unformattedText = text.message ? text.message : text; const formattedText = typeof unformattedText === 'object' ? JSON.stringify(text, null, 2) : text; - const textLength = previewTextLength || 140; + const textLength = previewTextLength ?? 140; const previewText = `${formattedText.substring(0, textLength)}${ formattedText.length > textLength ? ' ...' : '' }`; diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index f47dc885f5013..373dd436efef4 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -157,7 +157,7 @@ export const useDeleteTransforms = () => { const destinationIndex = status.destinationIndex; // if we are only deleting one transform, show the success toast messages - if (!isBulk) { + if (!isBulk && status.transformDeleted) { if (status.transformDeleted?.success) { toastNotifications.addSuccess( i18n.translate('xpack.transform.transformList.deleteTransformSuccessMessage', { diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx index 03dcb575f477f..d7db55990d333 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/action_delete.tsx @@ -15,6 +15,7 @@ import { EuiFlexGroup, EuiFlexItem, EuiSwitch, + EuiSpacer, } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; import { TRANSFORM_STATE } from '../../../../../../common'; @@ -99,11 +100,10 @@ export const DeleteAction: FC = ({ items, forceDisable }) => { = ({ items, forceDisable }) => /> } + { = ({ items, forceDisable }) => {userCanDeleteIndex && ( = ({ items, forceDisable }) => /> )} - - {userCanDeleteIndex && indexPatternExists && ( + {userCanDeleteIndex && indexPatternExists && ( + + = ({ items, forceDisable }) => checked={deleteIndexPattern} onChange={toggleDeleteIndexPattern} /> - )} - + + )} ); From def2ccfad563f00c7b45376087649cc2b000502c Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 5 Jun 2020 13:42:55 -0500 Subject: [PATCH 14/17] [Transform] Refactor new IndexService --- .../public/app/hooks/use_delete_transform.tsx | 29 +++------------- .../public/app/services/es_index_service.ts | 33 +++++++++++++++++++ .../transform/server/routes/api/transforms.ts | 2 +- 3 files changed, 38 insertions(+), 26 deletions(-) create mode 100644 x-pack/plugins/transform/public/app/services/es_index_service.ts diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index 373dd436efef4..1f395e67b7d31 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -6,7 +6,6 @@ import React, { useCallback, useEffect, useState } from 'react'; import { i18n } from '@kbn/i18n'; -import { CoreSetup } from 'kibana/public'; import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public'; import { TransformEndpointRequest, @@ -18,16 +17,7 @@ import { useAppDependencies, useToastNotifications } from '../app_dependencies'; import { TransformListRow, refreshTransformList$, REFRESH_TRANSFORM_LIST_STATE } from '../common'; import { ToastNotificationText } from '../components'; import { useApi } from './use_api'; -import { API_BASE_PATH } from '../../../common/constants'; -import { IIndexPattern } from '../../../../../../src/plugins/data/common/index_patterns'; - -export const canDeleteIndex = async (http: CoreSetup['http']) => { - const privilege = await http.get(`${API_BASE_PATH}privileges`); - if (!privilege) { - return false; - } - return privilege.hasAllPrivileges; -}; +import { indexService } from '../services/es_index_service'; export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { const { http, savedObjects } = useAppDependencies(); @@ -43,22 +33,11 @@ export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { const toggleDeleteIndexPattern = useCallback(() => setDeleteIndexPattern(!deleteIndexPattern), [ deleteIndexPattern, ]); - const savedObjectsClient = savedObjects.client; const checkIndexPatternExists = useCallback( async (indexName: string) => { try { - const response = await savedObjectsClient.find({ - type: 'index-pattern', - perPage: 10, - search: `"${indexName}"`, - searchFields: ['title'], - fields: ['title'], - }); - const ip = response.savedObjects.find( - (obj) => obj.attributes.title.toLowerCase() === indexName.toLowerCase() - ); - if (ip !== undefined) { + if (await indexService.indexPatternExists(savedObjects.client, indexName)) { setIndexPatternExists(true); } } catch (e) { @@ -76,12 +55,12 @@ export const useDeleteIndexAndTargetIndex = (items: TransformListRow[]) => { ); } }, - [savedObjectsClient, toastNotifications] + [savedObjects.client, toastNotifications] ); const checkUserIndexPermission = useCallback(async () => { try { - const userCanDelete = await canDeleteIndex(http); + const userCanDelete = await indexService.canDeleteIndex(http); if (userCanDelete) { setUserCanDeleteIndex(true); } diff --git a/x-pack/plugins/transform/public/app/services/es_index_service.ts b/x-pack/plugins/transform/public/app/services/es_index_service.ts new file mode 100644 index 0000000000000..491213d0ddbe7 --- /dev/null +++ b/x-pack/plugins/transform/public/app/services/es_index_service.ts @@ -0,0 +1,33 @@ +/* + * 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 { HttpSetup, SavedObjectsClientContract } from 'kibana/public'; +import { API_BASE_PATH } from '../../../common/constants'; +import { IIndexPattern } from '../../../../../../src/plugins/data/common/index_patterns'; + +export class IndexService { + async canDeleteIndex(http: HttpSetup) { + const privilege = await http.get(`${API_BASE_PATH}privileges`); + if (!privilege) { + return false; + } + return privilege.hasAllPrivileges; + } + + async indexPatternExists(savedObjectsClient: SavedObjectsClientContract, indexName: string) { + const response = await savedObjectsClient.find({ + type: 'index-pattern', + perPage: 1, + search: `"${indexName}"`, + searchFields: ['title'], + fields: ['title'], + }); + const ip = response.savedObjects.find((obj) => obj.attributes.title === indexName); + return ip !== undefined; + } +} + +export const indexService = new IndexService(); diff --git a/x-pack/plugins/transform/server/routes/api/transforms.ts b/x-pack/plugins/transform/server/routes/api/transforms.ts index 0557ccaa2a8d3..93fda56d319ad 100644 --- a/x-pack/plugins/transform/server/routes/api/transforms.ts +++ b/x-pack/plugins/transform/server/routes/api/transforms.ts @@ -275,7 +275,7 @@ async function getIndexPatternId( ) { const response = await savedObjectsClient.find({ type: 'index-pattern', - perPage: 10, + perPage: 1, search: `"${indexName}"`, searchFields: ['title'], fields: ['title'], From f2b70fb5e87d0d595c4719b42c1b531f10507016 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 9 Jun 2020 09:20:27 -0500 Subject: [PATCH 15/17] [Transform] Extend MLResponseError in extractErrorMessage --- .../ml/public/application/util/error_utils.ts | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/ml/public/application/util/error_utils.ts b/x-pack/plugins/ml/public/application/util/error_utils.ts index 427bf973d44dd..6d9e747bf2399 100644 --- a/x-pack/plugins/ml/public/application/util/error_utils.ts +++ b/x-pack/plugins/ml/public/application/util/error_utils.ts @@ -6,9 +6,20 @@ import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; +// Adding temporary types until Kibana ResponseError is updated +type MLResponseError = + | ResponseError + | { + message: { + msg: string; + }; + } + | { msg: string }; + export const extractErrorMessage = ( - error: CustomHttpResponseOptions | undefined | string + error: CustomHttpResponseOptions | undefined | string ): string | undefined => { + // extract only the error message within the response error coming from Kibana, Elasticsearch, and our own ML messages if (typeof error === 'string') { return error; } @@ -21,16 +32,12 @@ export const extractErrorMessage = ( if (typeof error.body.message === 'string') { return error.body.message; } - // @ts-ignore - if (typeof (error.body.message?.msg === 'string')) { - // @ts-ignore - return error.body.message?.msg; + if (typeof (error.body.message.msg === 'string')) { + return error.body.message.msg; } } if (typeof error.body === 'object' && 'msg' in error.body) { - // @ts-ignore if (typeof error.body.msg === 'string') { - // @ts-ignore return error.body.msg; } } From a5fda7f61b1ba11f3b4141b9c67f27d34b358e90 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 9 Jun 2020 16:16:16 -0500 Subject: [PATCH 16/17] [Transform] Add unit tests extractErrorMessage and modify type defs --- .../application/util/error_utils.test.ts | 53 +++++++++++++++++++ .../ml/public/application/util/error_utils.ts | 49 ++++++++++------- 2 files changed, 82 insertions(+), 20 deletions(-) create mode 100644 x-pack/plugins/ml/public/application/util/error_utils.test.ts diff --git a/x-pack/plugins/ml/public/application/util/error_utils.test.ts b/x-pack/plugins/ml/public/application/util/error_utils.test.ts new file mode 100644 index 0000000000000..a37cae42be518 --- /dev/null +++ b/x-pack/plugins/ml/public/application/util/error_utils.test.ts @@ -0,0 +1,53 @@ +/* + * 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 { extractErrorMessage, MLCustomHttpResponseOptions, MLResponseError } from './error_utils'; +import expect from '@kbn/expect/expect.js'; +import { ResponseError } from 'kibana/server'; + +describe('ML - error message utils', () => { + describe('extractErrorMessage', () => { + test('returns just the error message', () => { + const bodyWithNestedErrorMsg: MLCustomHttpResponseOptions = { + body: { + message: { + msg: 'Not Found', + }, + }, + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithNestedErrorMsg)).to.be('Not Found'); + + const bodyWithStringMsg: MLCustomHttpResponseOptions = { + body: { + msg: 'Not Found', + }, + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithStringMsg)).to.be('Not Found'); + + const bodyWithStringMessage: MLCustomHttpResponseOptions = { + body: { + message: 'Not Found', + }, + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithStringMessage)).to.be('Not Found'); + + const bodyWithString: MLCustomHttpResponseOptions = { + body: 'Not Found', + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithString)).to.be('Not Found'); + + const bodyWithError: MLCustomHttpResponseOptions = { + body: new Error('Not Found'), + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithError)).to.be('Not Found'); + }); + }); +}); diff --git a/x-pack/plugins/ml/public/application/util/error_utils.ts b/x-pack/plugins/ml/public/application/util/error_utils.ts index 6d9e747bf2399..c7de4472ca9dc 100644 --- a/x-pack/plugins/ml/public/application/util/error_utils.ts +++ b/x-pack/plugins/ml/public/application/util/error_utils.ts @@ -4,11 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; +import { ResponseError, ResponseHeaders } from 'kibana/server'; // Adding temporary types until Kibana ResponseError is updated -type MLResponseError = - | ResponseError +export type MLResponseError = | { message: { msg: string; @@ -16,31 +15,41 @@ type MLResponseError = } | { msg: string }; +export interface MLCustomHttpResponseOptions { + /** HTTP message to send to the client */ + body?: T; + /** HTTP Headers with additional information about response */ + headers?: ResponseHeaders; + statusCode: number; +} + export const extractErrorMessage = ( - error: CustomHttpResponseOptions | undefined | string + error: + | MLCustomHttpResponseOptions + | MLCustomHttpResponseOptions + | undefined + | string ): string | undefined => { // extract only the error message within the response error coming from Kibana, Elasticsearch, and our own ML messages + if (typeof error === 'string') { return error; } + if (error?.body === undefined) return; - if (error?.body) { - if (typeof error.body === 'string') { - return error.body; - } - if (typeof error.body === 'object' && 'message' in error.body) { - if (typeof error.body.message === 'string') { - return error.body.message; - } - if (typeof (error.body.message.msg === 'string')) { - return error.body.message.msg; - } + if (typeof error.body === 'string') { + return error.body; + } + if (typeof error.body === 'object' && 'msg' in error.body && typeof error.body.msg === 'string') { + return error.body.msg; + } + + if (typeof error.body === 'object' && 'message' in error.body) { + if (typeof error.body.message === 'string') { + return error.body.message; } - if (typeof error.body === 'object' && 'msg' in error.body) { - if (typeof error.body.msg === 'string') { - return error.body.msg; - } + if (!(error.body.message instanceof Error) && typeof (error.body.message.msg === 'string')) { + return error.body.message.msg; } } - return undefined; }; From 6f0fc961e68fd6ea0877b546cde25320b4727f62 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 9 Jun 2020 16:58:01 -0500 Subject: [PATCH 17/17] [Transform] Refactor extractErrorMessage to account for boom responses --- x-pack/plugins/ml/common/util/errors.test.ts | 78 ++++++++++++++++ x-pack/plugins/ml/common/util/errors.ts | 88 ++++++++++++++----- .../analytics_list/action_delete.tsx | 2 +- .../analytics_service/delete_analytics.ts | 2 +- .../application/util/error_utils.test.ts | 53 ----------- .../ml/public/application/util/error_utils.ts | 55 ------------ 6 files changed, 145 insertions(+), 133 deletions(-) create mode 100644 x-pack/plugins/ml/common/util/errors.test.ts delete mode 100644 x-pack/plugins/ml/public/application/util/error_utils.test.ts delete mode 100644 x-pack/plugins/ml/public/application/util/error_utils.ts diff --git a/x-pack/plugins/ml/common/util/errors.test.ts b/x-pack/plugins/ml/common/util/errors.test.ts new file mode 100644 index 0000000000000..00af27248ccce --- /dev/null +++ b/x-pack/plugins/ml/common/util/errors.test.ts @@ -0,0 +1,78 @@ +/* + * 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 { + BoomResponse, + extractErrorMessage, + MLCustomHttpResponseOptions, + MLResponseError, +} from './errors'; +import { ResponseError } from 'kibana/server'; + +describe('ML - error message utils', () => { + describe('extractErrorMessage', () => { + test('returns just the error message', () => { + const testMsg = 'Saved object [index-pattern/blahblahblah] not found'; + + const bodyWithNestedErrorMsg: MLCustomHttpResponseOptions = { + body: { + message: { + msg: testMsg, + }, + }, + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithNestedErrorMsg)).toBe(testMsg); + + const bodyWithStringMsg: MLCustomHttpResponseOptions = { + body: { + msg: testMsg, + }, + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithStringMsg)).toBe(testMsg); + + const bodyWithStringMessage: MLCustomHttpResponseOptions = { + body: { + message: testMsg, + }, + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithStringMessage)).toBe(testMsg); + + const bodyWithString: MLCustomHttpResponseOptions = { + body: testMsg, + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithString)).toBe(testMsg); + + const bodyWithError: MLCustomHttpResponseOptions = { + body: new Error(testMsg), + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithError)).toBe(testMsg); + + const bodyWithBoomError: MLCustomHttpResponseOptions = { + statusCode: 404, + body: { + data: [], + isBoom: true, + isServer: false, + output: { + statusCode: 404, + payload: { + statusCode: 404, + error: testMsg, + message: testMsg, + }, + headers: {}, + }, + }, + }; + expect(extractErrorMessage(bodyWithBoomError)).toBe(testMsg); + }); + }); +}); diff --git a/x-pack/plugins/ml/common/util/errors.ts b/x-pack/plugins/ml/common/util/errors.ts index 6f620f74a4531..e165e15d7c64e 100644 --- a/x-pack/plugins/ml/common/util/errors.ts +++ b/x-pack/plugins/ml/common/util/errors.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; +import { ResponseError, ResponseHeaders } from 'kibana/server'; import { isErrorResponse } from '../types/errors'; export function getErrorMessage(error: any) { @@ -19,34 +19,76 @@ export function getErrorMessage(error: any) { return JSON.stringify(error); } +// Adding temporary types until Kibana ResponseError is updated + +export interface BoomResponse { + data: any; + isBoom: boolean; + isServer: boolean; + output: { + statusCode: number; + payload: { + statusCode: number; + error: string; + message: string; + }; + headers: {}; + }; +} +export type MLResponseError = + | { + message: { + msg: string; + }; + } + | { msg: string }; + +export interface MLCustomHttpResponseOptions< + T extends ResponseError | MLResponseError | BoomResponse +> { + /** HTTP message to send to the client */ + body?: T; + /** HTTP Headers with additional information about response */ + headers?: ResponseHeaders; + statusCode: number; +} + export const extractErrorMessage = ( - error: CustomHttpResponseOptions | undefined | string -): string | undefined => { + error: + | MLCustomHttpResponseOptions + | undefined + | string +): string => { + // extract only the error message within the response error coming from Kibana, Elasticsearch, and our own ML messages + if (typeof error === 'string') { return error; } + if (error?.body === undefined) return ''; - if (error?.body) { - if (typeof error.body === 'string') { - return error.body; - } - if (typeof error.body === 'object' && 'message' in error.body) { - if (typeof error.body.message === 'string') { - return error.body.message; - } - // @ts-ignore - if (typeof (error.body.message?.msg === 'string')) { - // @ts-ignore - return error.body.message?.msg; - } + if (typeof error.body === 'string') { + return error.body; + } + if ( + typeof error.body === 'object' && + 'output' in error.body && + error.body.output.payload.message + ) { + return error.body.output.payload.message; + } + + if (typeof error.body === 'object' && 'msg' in error.body && typeof error.body.msg === 'string') { + return error.body.msg; + } + + if (typeof error.body === 'object' && 'message' in error.body) { + if (typeof error.body.message === 'string') { + return error.body.message; } - if (typeof error.body === 'object' && 'msg' in error.body) { - // @ts-ignore - if (typeof error.body.msg === 'string') { - // @ts-ignore - return error.body.msg; - } + if (!(error.body.message instanceof Error) && typeof (error.body.message.msg === 'string')) { + return error.body.message.msg; } } - return undefined; + // If all else fail return an empty message instead of JSON.stringify + return ''; }; diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_delete.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_delete.tsx index 2d433f6b18484..38ef00914e8fb 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_delete.tsx +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_delete.tsx @@ -18,6 +18,7 @@ import { } from '@elastic/eui'; import { IIndexPattern } from 'src/plugins/data/common'; import { FormattedMessage } from '@kbn/i18n/react'; +import { extractErrorMessage } from '../../../../../../../common/util/errors'; import { deleteAnalytics, deleteAnalyticsAndDestIndex, @@ -29,7 +30,6 @@ import { } from '../../../../../capabilities/check_capabilities'; import { useMlKibana } from '../../../../../contexts/kibana'; import { isDataFrameAnalyticsRunning, DataFrameAnalyticsListRow } from './common'; -import { extractErrorMessage } from '../../../../../util/error_utils'; interface DeleteActionProps { item: DataFrameAnalyticsListRow; diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/services/analytics_service/delete_analytics.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/services/analytics_service/delete_analytics.ts index 26cefff0a3f59..ebd3fa8982604 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/services/analytics_service/delete_analytics.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/services/analytics_service/delete_analytics.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ import { i18n } from '@kbn/i18n'; +import { extractErrorMessage } from '../../../../../../../common/util/errors'; import { getToastNotifications } from '../../../../../util/dependency_cache'; import { ml } from '../../../../../services/ml_api_service'; import { refreshAnalyticsList$, REFRESH_ANALYTICS_LIST_STATE } from '../../../../common'; @@ -11,7 +12,6 @@ import { isDataFrameAnalyticsFailed, DataFrameAnalyticsListRow, } from '../../components/analytics_list/common'; -import { extractErrorMessage } from '../../../../../util/error_utils'; export const deleteAnalytics = async (d: DataFrameAnalyticsListRow) => { const toastNotifications = getToastNotifications(); diff --git a/x-pack/plugins/ml/public/application/util/error_utils.test.ts b/x-pack/plugins/ml/public/application/util/error_utils.test.ts deleted file mode 100644 index a37cae42be518..0000000000000 --- a/x-pack/plugins/ml/public/application/util/error_utils.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -/* - * 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 { extractErrorMessage, MLCustomHttpResponseOptions, MLResponseError } from './error_utils'; -import expect from '@kbn/expect/expect.js'; -import { ResponseError } from 'kibana/server'; - -describe('ML - error message utils', () => { - describe('extractErrorMessage', () => { - test('returns just the error message', () => { - const bodyWithNestedErrorMsg: MLCustomHttpResponseOptions = { - body: { - message: { - msg: 'Not Found', - }, - }, - statusCode: 404, - }; - expect(extractErrorMessage(bodyWithNestedErrorMsg)).to.be('Not Found'); - - const bodyWithStringMsg: MLCustomHttpResponseOptions = { - body: { - msg: 'Not Found', - }, - statusCode: 404, - }; - expect(extractErrorMessage(bodyWithStringMsg)).to.be('Not Found'); - - const bodyWithStringMessage: MLCustomHttpResponseOptions = { - body: { - message: 'Not Found', - }, - statusCode: 404, - }; - expect(extractErrorMessage(bodyWithStringMessage)).to.be('Not Found'); - - const bodyWithString: MLCustomHttpResponseOptions = { - body: 'Not Found', - statusCode: 404, - }; - expect(extractErrorMessage(bodyWithString)).to.be('Not Found'); - - const bodyWithError: MLCustomHttpResponseOptions = { - body: new Error('Not Found'), - statusCode: 404, - }; - expect(extractErrorMessage(bodyWithError)).to.be('Not Found'); - }); - }); -}); diff --git a/x-pack/plugins/ml/public/application/util/error_utils.ts b/x-pack/plugins/ml/public/application/util/error_utils.ts deleted file mode 100644 index c7de4472ca9dc..0000000000000 --- a/x-pack/plugins/ml/public/application/util/error_utils.ts +++ /dev/null @@ -1,55 +0,0 @@ -/* - * 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 { ResponseError, ResponseHeaders } from 'kibana/server'; - -// Adding temporary types until Kibana ResponseError is updated -export type MLResponseError = - | { - message: { - msg: string; - }; - } - | { msg: string }; - -export interface MLCustomHttpResponseOptions { - /** HTTP message to send to the client */ - body?: T; - /** HTTP Headers with additional information about response */ - headers?: ResponseHeaders; - statusCode: number; -} - -export const extractErrorMessage = ( - error: - | MLCustomHttpResponseOptions - | MLCustomHttpResponseOptions - | undefined - | string -): string | undefined => { - // extract only the error message within the response error coming from Kibana, Elasticsearch, and our own ML messages - - if (typeof error === 'string') { - return error; - } - if (error?.body === undefined) return; - - if (typeof error.body === 'string') { - return error.body; - } - if (typeof error.body === 'object' && 'msg' in error.body && typeof error.body.msg === 'string') { - return error.body.msg; - } - - if (typeof error.body === 'object' && 'message' in error.body) { - if (typeof error.body.message === 'string') { - return error.body.message; - } - if (!(error.body.message instanceof Error) && typeof (error.body.message.msg === 'string')) { - return error.body.message.msg; - } - } -};