[Enterprise Search] Disallow creating pipeline with placeholder of mo…

…del (elastic#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: <img width="919" alt="Screenshot 2023-12-08 at 14 03 59" src="https://github.com/elastic/kibana/assets/14224983/26f8be24-3f78-4765-9a4c-771adbb221a4"> Invalid selection: <img width="921" alt="Screenshot 2023-12-08 at 14 04 04" src="https://github.com/elastic/kibana/assets/14224983/a157fb69-c004-49b2-93c7-13b58e82074c"> ### 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 <[email protected]>
davismcphee · Dec 12, 2023 · 156c8f2 · 156c8f2
1 parent 9502512
commit 156c8f2
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 9 deletions.
diff --git a/...rise_search_content/components/search_index/pipelines/ml_inference/configure_pipeline.tsx b/...rise_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 = () => {
                 <EuiSpacer />
               </>
             )}
-            <EuiFormRow
-              fullWidth
-              label={i18n.translate(
-                'xpack.enterpriseSearch.content.indices.pipelines.addInferencePipelineModal.steps.configure.titleSelectTrainedModel',
-                { defaultMessage: 'Select a trained ML Model' }
-              )}
-            >
-              <ModelSelect />
-            </EuiFormRow>
+            <EuiSpacer size="s" />
+            <EuiTitle size="xxxs">
+              <h6>
+                {i18n.translate(
+                  'xpack.enterpriseSearch.content.indices.pipelines.addInferencePipelineModal.steps.configure.titleSelectTrainedModel',
+                  { defaultMessage: 'Select a trained ML Model' }
+                )}
+              </h6>
+            </EuiTitle>
+            {formErrors.modelStatus !== undefined && (
+              <>
+                <EuiSpacer size="xs" />
+                <EuiText size="xs">
+                  <p>
+                    <EuiTextColor color="danger">{formErrors.modelStatus}</EuiTextColor>
+                  </p>
+                </EuiText>
+              </>
+            )}
+            <EuiSpacer size="xs" />
+            <ModelSelect />
           </EuiForm>
         </>
       ),

diff --git a/..._search_content/components/search_index/pipelines/ml_inference/ml_inference_logic.test.ts b/..._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/...prise_search_content/components/search_index/pipelines/ml_inference/model_select.test.tsx b/...prise_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(<ModelSelect />);
+    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/...enterprise_search_content/components/search_index/pipelines/ml_inference/model_select.tsx b/...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/...cations/enterprise_search_content/components/search_index/pipelines/ml_inference/types.ts b/...cations/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/...cations/enterprise_search_content/components/search_index/pipelines/ml_inference/utils.ts b/...cations/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;