From 156c8f2323a65153397bbddd308bfd23901d50ba Mon Sep 17 00:00:00 2001 From: Adam Demjen Date: Tue, 12 Dec 2023 12:42:16 -0500 Subject: [PATCH] [Enterprise Search] Disallow creating pipeline with placeholder of model (#172988) ## Summary This PR adds validation to model selection when creating an inference pipeline. Pipelines cannot be created with a non-deployed placeholder of a model (e.g. E5, ELSER) as this would break other Search UIs and functionality. If such an item is selected, an error message is displayed and the Continue button is disabled. Note this feature is independent of the changing of the model selection component. Valid selection: Screenshot 2023-12-08 at 14 03 59 Invalid selection: Screenshot 2023-12-08 at 14 04 04 ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../ml_inference/configure_pipeline.tsx | 31 +++++++++++++------ .../ml_inference/ml_inference_logic.test.ts | 19 ++++++++++++ .../ml_inference/model_select.test.tsx | 17 ++++++++++ .../pipelines/ml_inference/model_select.tsx | 1 + .../pipelines/ml_inference/types.ts | 2 ++ .../pipelines/ml_inference/utils.ts | 8 +++++ 6 files changed, 69 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/configure_pipeline.tsx b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/configure_pipeline.tsx index cc318831555afd..52d4f38b454084 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/configure_pipeline.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/configure_pipeline.tsx @@ -19,6 +19,7 @@ import { EuiTabbedContentTab, EuiTitle, EuiText, + EuiTextColor, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; @@ -130,15 +131,27 @@ export const ConfigurePipeline: React.FC = () => { )} - - - + + +
+ {i18n.translate( + 'xpack.enterpriseSearch.content.indices.pipelines.addInferencePipelineModal.steps.configure.titleSelectTrainedModel', + { defaultMessage: 'Select a trained ML Model' } + )} +
+ + {formErrors.modelStatus !== undefined && ( + <> + + +

+ {formErrors.modelStatus} +

+ + + )} + + ), diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/ml_inference_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/ml_inference_logic.test.ts index a725371de02427..7412d861d7136a 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/ml_inference_logic.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/ml_inference_logic.test.ts @@ -550,6 +550,25 @@ describe('MlInferenceLogic', () => { pipelineName: 'Name already used by another pipeline.', }); }); + it('has errors when non-deployed model is selected', () => { + MLInferenceLogic.actions.setInferencePipelineConfiguration({ + ...MLInferenceLogic.values.addInferencePipelineModal.configuration, + pipelineName: 'unit-test-pipeline', + modelID: 'unit-test-model', + existingPipeline: false, + fieldMappings: [ + { + sourceField: 'body', + targetField: 'ml.inference.body', + }, + ], + isModelPlaceholderSelected: true, + }); + + expect(MLInferenceLogic.values.formErrors).toEqual({ + modelStatus: 'Model must be deployed before use.', + }); + }); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/model_select.test.tsx b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/model_select.test.tsx index 15fb492fae56d8..9bd006f65883e0 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/model_select.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/model_select.test.tsx @@ -98,6 +98,23 @@ describe('ModelSelect', () => { }) ); }); + it('sets placeholder flag on selecting a placeholder item', () => { + setMockValues(DEFAULT_VALUES); + + const wrapper = shallow(); + expect(wrapper.find(EuiSelectable)).toHaveLength(1); + const selectable = wrapper.find(EuiSelectable); + selectable.simulate('change', [ + { modelId: 'model_1' }, + { modelId: 'model_2', isPlaceholder: true, checked: 'on' }, + ]); + expect(MOCK_ACTIONS.setInferencePipelineConfiguration).toHaveBeenCalledWith( + expect.objectContaining({ + modelID: 'model_2', + isModelPlaceholderSelected: true, + }) + ); + }); it('generates pipeline name on selecting an item', () => { setMockValues(DEFAULT_VALUES); diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/model_select.tsx b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/model_select.tsx index 86c91c483702f3..ac3900b6ed6621 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/model_select.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/model_select.tsx @@ -44,6 +44,7 @@ export const ModelSelect: React.FC = () => { ...configuration, inferenceConfig: undefined, modelID: selectedOption?.modelId ?? '', + isModelPlaceholderSelected: selectedOption?.isPlaceholder ?? false, fieldMappings: undefined, pipelineName: isPipelineNameUserSupplied ? pipelineName diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/types.ts b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/types.ts index 3a645dcbba3b4c..87aed3eb4d714f 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/types.ts +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/types.ts @@ -13,6 +13,7 @@ export interface InferencePipelineConfiguration { existingPipeline?: boolean; inferenceConfig?: InferencePipelineInferenceConfig; isPipelineNameUserSupplied?: boolean; + isModelPlaceholderSelected?: boolean; modelID: string; pipelineName: string; fieldMappings?: FieldMapping[]; @@ -21,6 +22,7 @@ export interface InferencePipelineConfiguration { export interface AddInferencePipelineFormErrors { modelID?: string; + modelStatus?: string; fieldMappings?: string; pipelineName?: string; } diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/utils.ts b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/utils.ts index 02cf5a7463ddeb..5a01a3823a71df 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/utils.ts +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/pipelines/ml_inference/utils.ts @@ -37,6 +37,12 @@ const PIPELINE_NAME_EXISTS_ERROR = i18n.translate( defaultMessage: 'Name already used by another pipeline.', } ); +const MODEL_NOT_DEPLOYED_ERROR = i18n.translate( + 'xpack.enterpriseSearch.content.indices.pipelines.addInferencePipelineModal.steps.configure.modelNotDeployedError', + { + defaultMessage: 'Model must be deployed before use.', + } +); export const validateInferencePipelineConfiguration = ( config: InferencePipelineConfiguration @@ -55,6 +61,8 @@ export const validateInferencePipelineConfiguration = ( } if (config.modelID.trim().length === 0) { errors.modelID = FIELD_REQUIRED_ERROR; + } else if (config.isModelPlaceholderSelected) { + errors.modelStatus = MODEL_NOT_DEPLOYED_ERROR; } return errors;