[Ingest Pipelines] Preserve unknown fields in processors (#91146) (#9…

…1630) * keep known and unknown options in processor config * added test for preserving unknown fields * refactor form for NOT stripping empty field values, also allow empty "value" for set and gsub * remove unused i18n * fix user agent form serialization, update field help text * remove out of date translation Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Feb 17, 2021 · a54063b · a54063b
1 parent c70a8a0
commit a54063b
Show file tree

Hide file tree

Showing 22 changed files with 154 additions and 73 deletions.
diff --git a/...ic/application/components/pipeline_editor/__jest__/pipeline_processors_editor.helpers.tsx b/...ic/application/components/pipeline_editor/__jest__/pipeline_processors_editor.helpers.tsx
@@ -154,6 +154,17 @@ const createActions = (testBed: TestBed<TestSubject>) => {
         find(`${processorSelector}.moreMenu.duplicateButton`).simulate('click');
       });
     },
+    openProcessorEditor: (processorSelector: string) => {
+      act(() => {
+        find(`${processorSelector}.manageItemButton`).simulate('click');
+      });
+      component.update();
+    },
+    submitProcessorForm: async () => {
+      await act(async () => {
+        find('editProcessorForm.submitButton').simulate('click');
+      });
+    },
   };
 };
 

diff --git a/...ublic/application/components/pipeline_editor/__jest__/pipeline_processors_editor.test.tsx b/...ublic/application/components/pipeline_editor/__jest__/pipeline_processors_editor.test.tsx
@@ -22,6 +22,13 @@ const testProcessors: Pick<Pipeline, 'processors'> = {
         replacement: '$17$2',
       },
     },
+    {
+      set: {
+        field: 'test',
+        value: 'test',
+        unknown_field_foo: 'unknown_value',
+      },
+    },
   ],
 };
 
@@ -79,11 +86,37 @@ describe('Pipeline Editor', () => {
       await actions.addProcessor('processors', 'test', { if: '1 == 1' });
       const [onUpdateResult] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const { processors } = onUpdateResult.getData();
-      expect(processors.length).toBe(3);
-      const [a, b, c] = processors;
+      expect(processors.length).toBe(4);
+      const [a, b, c, d] = processors;
       expect(a).toEqual(testProcessors.processors[0]);
       expect(b).toEqual(testProcessors.processors[1]);
-      expect(c).toEqual({ test: { if: '1 == 1' } });
+      expect(c).toEqual(testProcessors.processors[2]);
+      expect(d).toEqual({ test: { if: '1 == 1' } });
+    });
+
+    it('edits a processor without removing unknown processor.options', async () => {
+      const { actions, exists, form } = testBed;
+      // Open the edit processor form for the set processor
+      actions.openProcessorEditor('processors>2');
+      expect(exists('editProcessorForm')).toBeTruthy();
+      form.setInputValue('editProcessorForm.valueFieldInput', 'test44');
+      await actions.submitProcessorForm();
+      const [onUpdateResult] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
+      const {
+        processors: { 2: setProcessor },
+      } = onUpdateResult.getData();
+      // The original field should still be unchanged
+      expect(testProcessors.processors[2].set.value).toBe('test');
+      expect(setProcessor.set).toEqual({
+        description: undefined,
+        field: 'test',
+        ignore_empty_value: undefined,
+        ignore_failure: undefined,
+        override: undefined,
+        // This unknown_field is not supported in the form
+        unknown_field_foo: 'unknown_value',
+        value: 'test44',
+      });
     });
 
     it('removes a processor', () => {
@@ -92,7 +125,7 @@ describe('Pipeline Editor', () => {
       actions.removeProcessor('processors>0');
       const [onUpdateResult] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const { processors } = onUpdateResult.getData();
-      expect(processors.length).toBe(1);
+      expect(processors.length).toBe(2);
       expect(processors[0]).toEqual({
         gsub: {
           field: '_index',
@@ -107,7 +140,11 @@ describe('Pipeline Editor', () => {
       actions.moveProcessor('processors>0', 'dropButtonBelow-processors>1');
       const [onUpdateResult] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const { processors } = onUpdateResult.getData();
-      expect(processors).toEqual(testProcessors.processors.slice(0).reverse());
+      expect(processors).toEqual([
+        testProcessors.processors[1],
+        testProcessors.processors[0],
+        testProcessors.processors[2],
+      ]);
     });
 
     it('adds an on-failure processor to a processor', async () => {
@@ -121,7 +158,7 @@ describe('Pipeline Editor', () => {
       expect(exists(`${processorSelector}.addProcessor`));
       const [onUpdateResult] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const { processors } = onUpdateResult.getData();
-      expect(processors.length).toBe(2);
+      expect(processors.length).toBe(3);
       expect(processors[0]).toEqual(testProcessors.processors[0]); // should be unchanged
       expect(processors[1].gsub).toEqual({
         ...testProcessors.processors[1].gsub,
@@ -135,7 +172,7 @@ describe('Pipeline Editor', () => {
       actions.moveProcessor('processors>0', 'dropButtonBelow-processors>1>onFailure>0');
       const [onUpdateResult] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const { processors } = onUpdateResult.getData();
-      expect(processors.length).toBe(1);
+      expect(processors.length).toBe(2);
       expect(processors[0].gsub.on_failure).toEqual([
         {
           test: { if: '1 == 3' },
@@ -150,7 +187,7 @@ describe('Pipeline Editor', () => {
       actions.duplicateProcessor('processors>1');
       const [onUpdateResult] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const { processors } = onUpdateResult.getData();
-      expect(processors.length).toBe(3);
+      expect(processors.length).toBe(4);
       const duplicatedProcessor = {
         gsub: {
           ...testProcessors.processors[1].gsub,
@@ -161,6 +198,7 @@ describe('Pipeline Editor', () => {
         testProcessors.processors[0],
         duplicatedProcessor,
         duplicatedProcessor,
+        testProcessors.processors[2],
       ]);
     });
 
@@ -182,14 +220,17 @@ describe('Pipeline Editor', () => {
       actions.moveProcessor('processors>0', 'dropButtonBelow-onFailure>0');
       const [onUpdateResult1] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const data1 = onUpdateResult1.getData();
-      expect(data1.processors.length).toBe(1);
+      expect(data1.processors.length).toBe(2);
       expect(data1.on_failure.length).toBe(2);
-      expect(data1.processors).toEqual([testProcessors.processors[1]]);
+      expect(data1.processors).toEqual([
+        testProcessors.processors[1],
+        testProcessors.processors[2],
+      ]);
       expect(data1.on_failure).toEqual([{ test: { if: '1 == 5' } }, testProcessors.processors[0]]);
       actions.moveProcessor('onFailure>1', 'dropButtonAbove-processors>0');
       const [onUpdateResult2] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const data2 = onUpdateResult2.getData();
-      expect(data2.processors.length).toBe(2);
+      expect(data2.processors.length).toBe(3);
       expect(data2.on_failure.length).toBe(1);
       expect(data2.processors).toEqual(testProcessors.processors);
       expect(data2.on_failure).toEqual([{ test: { if: '1 == 5' } }]);
@@ -208,7 +249,7 @@ describe('Pipeline Editor', () => {
       actions.moveProcessor('processors>0', 'onFailure.dropButtonEmptyTree');
       const [onUpdateResult2] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const data = onUpdateResult2.getData();
-      expect(data.processors).toEqual([testProcessors.processors[1]]);
+      expect(data.processors).toEqual([testProcessors.processors[1], testProcessors.processors[2]]);
       expect(data.on_failure).toEqual([testProcessors.processors[0]]);
     });
   });

diff --git a/...ication/components/pipeline_editor/components/processor_form/processor_form.container.tsx b/...ication/components/pipeline_editor/components/processor_form/processor_form.container.tsx
@@ -7,7 +7,13 @@
 
 import React, { FunctionComponent, useCallback, useEffect, useRef } from 'react';
 
-import { useForm, OnFormUpdateArg, FormData, useKibana } from '../../../../../shared_imports';
+import {
+  useForm,
+  OnFormUpdateArg,
+  FormData,
+  FormOptions,
+  useKibana,
+} from '../../../../../shared_imports';
 import { ProcessorInternal } from '../../types';
 
 import { EditProcessorForm } from './edit_processor_form';
@@ -33,6 +39,14 @@ interface Props {
   processor?: ProcessorInternal;
 }
 
+const formOptions: FormOptions = {
+  /**
+   * This is important for allowing configuration of empty text fields in certain processors that
+   * remove values from their inputs.
+   */
+  stripEmptyFields: false,
+};
+
 export const ProcessorFormContainer: FunctionComponent<Props> = ({
   processor,
   onFormUpdate,
@@ -81,6 +95,7 @@ export const ProcessorFormContainer: FunctionComponent<Props> = ({
   const { form } = useForm({
     defaultValue: { fields: getProcessor().options },
     serializer: formSerializer,
+    options: formOptions,
   });
   const { subscribe } = form;
 

diff --git a/...ine_editor/components/processor_form/processors/common_fields/common_processor_fields.tsx b/...ine_editor/components/processor_form/processors/common_fields/common_processor_fields.tsx
@@ -38,6 +38,7 @@ const ignoreFailureConfig: FieldConfig<any> = {
 };
 
 const ifConfig: FieldConfig = {
+  serializer: from.emptyStringToUndefined,
   label: i18n.translate('xpack.ingestPipelines.pipelineEditor.commonFields.ifFieldLabel', {
     defaultMessage: 'Condition (optional)',
   }),
@@ -48,6 +49,7 @@ const ifConfig: FieldConfig = {
 };
 
 const tagConfig: FieldConfig = {
+  serializer: from.emptyStringToUndefined,
   label: i18n.translate('xpack.ingestPipelines.pipelineEditor.commonFields.tagFieldLabel', {
     defaultMessage: 'Tag (optional)',
   }),

diff --git a/...nents/pipeline_editor/components/processor_form/processors/common_fields/target_field.tsx b/...nents/pipeline_editor/components/processor_form/processors/common_fields/target_field.tsx
@@ -10,12 +10,13 @@ import { i18n } from '@kbn/i18n';
 
 import { Field, FIELD_TYPES, UseField, FieldConfig } from '../../../../../../../shared_imports';
 
-import { FieldsConfig } from '../shared';
+import { FieldsConfig, from } from '../shared';
 
 const fieldsConfig: FieldsConfig = {
   target_field: {
     type: FIELD_TYPES.TEXT,
     deserializer: String,
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate('xpack.ingestPipelines.pipelineEditor.commonFields.targetFieldLabel', {
       defaultMessage: 'Target field (optional)',
     }),

diff --git a/...ublic/application/components/pipeline_editor/components/processor_form/processors/csv.tsx b/...ublic/application/components/pipeline_editor/components/processor_form/processors/csv.tsx
@@ -24,7 +24,7 @@ import { FieldsConfig } from './shared';
 import { IgnoreMissingField } from './common_fields/ignore_missing_field';
 import { FieldNameField } from './common_fields/field_name_field';
 
-import { to } from './shared';
+import { to, from } from './shared';
 
 const { minLengthField } = fieldValidators;
 
@@ -72,7 +72,7 @@ const fieldsConfig: FieldsConfig = {
   /* Optional fields config */
   separator: {
     type: FIELD_TYPES.TEXT,
-    serializer: (v) => (v ? v : undefined),
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate('xpack.ingestPipelines.pipelineEditor.convertForm.separatorFieldLabel', {
       defaultMessage: 'Separator (optional)',
     }),
@@ -91,7 +91,7 @@ const fieldsConfig: FieldsConfig = {
   },
   quote: {
     type: FIELD_TYPES.TEXT,
-    serializer: (v) => (v ? v : undefined),
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate('xpack.ingestPipelines.pipelineEditor.convertForm.quoteFieldLabel', {
       defaultMessage: 'Quote (optional)',
     }),
@@ -121,6 +121,7 @@ const fieldsConfig: FieldsConfig = {
   },
   empty_value: {
     type: FIELD_TYPES.TEXT,
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate('xpack.ingestPipelines.pipelineEditor.convertForm.emptyValueFieldLabel', {
       defaultMessage: 'Empty value (optional)',
     }),

diff --git a/...blic/application/components/pipeline_editor/components/processor_form/processors/date.tsx b/...blic/application/components/pipeline_editor/components/processor_form/processors/date.tsx
@@ -18,7 +18,7 @@ import {
   ComboBoxField,
 } from '../../../../../../shared_imports';
 
-import { FieldsConfig, to } from './shared';
+import { FieldsConfig, to, from } from './shared';
 import { FieldNameField } from './common_fields/field_name_field';
 import { TargetField } from './common_fields/target_field';
 
@@ -53,7 +53,7 @@ const fieldsConfig: FieldsConfig = {
   /* Optional fields config */
   timezone: {
     type: FIELD_TYPES.TEXT,
-    serializer: (v) => (v ? v : undefined),
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate('xpack.ingestPipelines.pipelineEditor.dateForm.timezoneFieldLabel', {
       defaultMessage: 'Timezone (optional)',
     }),
@@ -67,7 +67,7 @@ const fieldsConfig: FieldsConfig = {
   },
   locale: {
     type: FIELD_TYPES.TEXT,
-    serializer: (v) => (v ? v : undefined),
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate('xpack.ingestPipelines.pipelineEditor.dateForm.localeFieldLabel', {
       defaultMessage: 'Locale (optional)',
     }),

diff --git a/...ation/components/pipeline_editor/components/processor_form/processors/date_index_name.tsx b/...ation/components/pipeline_editor/components/processor_form/processors/date_index_name.tsx
@@ -19,7 +19,7 @@ import {
   SelectField,
 } from '../../../../../../shared_imports';
 
-import { FieldsConfig, to } from './shared';
+import { FieldsConfig, to, from } from './shared';
 import { FieldNameField } from './common_fields/field_name_field';
 
 const { emptyField } = fieldValidators;
@@ -57,7 +57,7 @@ const fieldsConfig: FieldsConfig = {
   /* Optional fields config */
   index_name_prefix: {
     type: FIELD_TYPES.TEXT,
-    serializer: (v) => (v ? v : undefined),
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate(
       'xpack.ingestPipelines.pipelineEditor.dateIndexNameForm.indexNamePrefixFieldLabel',
       {
@@ -71,7 +71,7 @@ const fieldsConfig: FieldsConfig = {
   },
   index_name_format: {
     type: FIELD_TYPES.TEXT,
-    serializer: (v) => (v ? v : undefined),
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate(
       'xpack.ingestPipelines.pipelineEditor.dateIndexNameForm.indexNameFormatFieldLabel',
       {
@@ -108,7 +108,7 @@ const fieldsConfig: FieldsConfig = {
   },
   timezone: {
     type: FIELD_TYPES.TEXT,
-    serializer: (v) => (v ? v : undefined),
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate(
       'xpack.ingestPipelines.pipelineEditor.dateIndexNameForm.timezoneFieldLabel',
       {
@@ -125,7 +125,7 @@ const fieldsConfig: FieldsConfig = {
   },
   locale: {
     type: FIELD_TYPES.TEXT,
-    serializer: (v) => (v ? v : undefined),
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate(
       'xpack.ingestPipelines.pipelineEditor.dateIndexNameForm.localeFieldLabel',
       {

diff --git a/...c/application/components/pipeline_editor/components/processor_form/processors/dissect.tsx b/...c/application/components/pipeline_editor/components/processor_form/processors/dissect.tsx
@@ -22,7 +22,7 @@ import {
 
 import { FieldNameField } from './common_fields/field_name_field';
 import { IgnoreMissingField } from './common_fields/ignore_missing_field';
-import { EDITOR_PX_HEIGHT } from './shared';
+import { EDITOR_PX_HEIGHT, from } from './shared';
 
 const { emptyField } = fieldValidators;
 
@@ -72,6 +72,7 @@ const getFieldsConfig = (esDocUrl: string): Record<string, FieldConfig> => {
     /* Optional field config */
     append_separator: {
       type: FIELD_TYPES.TEXT,
+      serializer: from.emptyStringToUndefined,
       label: i18n.translate(
         'xpack.ingestPipelines.pipelineEditor.dissectForm.appendSeparatorparaotrFieldLabel',
         {

diff --git a/...lication/components/pipeline_editor/components/processor_form/processors/dot_expander.tsx b/...lication/components/pipeline_editor/components/processor_form/processors/dot_expander.tsx
@@ -12,9 +12,12 @@ import { FieldConfig, FIELD_TYPES, UseField, Field } from '../../../../../../sha
 
 import { FieldNameField } from './common_fields/field_name_field';
 
+import { from } from './shared';
+
 const fieldsConfig: Record<string, FieldConfig> = {
   path: {
     type: FIELD_TYPES.TEXT,
+    serializer: from.emptyStringToUndefined,
     label: i18n.translate('xpack.ingestPipelines.pipelineEditor.dotExpanderForm.pathFieldLabel', {
       defaultMessage: 'Path',
     }),

diff --git a/...lic/application/components/pipeline_editor/components/processor_form/processors/geoip.tsx b/...lic/application/components/pipeline_editor/components/processor_form/processors/geoip.tsx
@@ -22,7 +22,7 @@ const fieldsConfig: FieldsConfig = {
   /* Optional field config */
   database_file: {
     type: FIELD_TYPES.TEXT,
-    serializer: (v) => (v === 'GeoLite2-City.mmdb' ? undefined : v),
+    serializer: (v) => (v === 'GeoLite2-City.mmdb' || v === '' ? undefined : v),
     label: i18n.translate('xpack.ingestPipelines.pipelineEditor.geoIPForm.databaseFileLabel', {
       defaultMessage: 'Database file (optional)',
     }),