Skip to content

Commit

Permalink
[8.x] Create a common Int Validator and use it in ingest_pipelines an…
Browse files Browse the repository at this point in the history
…d Index_lifecycle_management (#196527) (#197685)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Create a common Int Validator and use it in ingest_pipelines and
Index_lifecycle_management
(#196527)](#196527)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Sonia Sanz
Vivas","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-24T16:09:38Z","message":"Create
a common Int Validator and use it in ingest_pipelines and
Index_lifecycle_management (#196527)\n\nCloses [#110417
](https://github.com/elastic/kibana/issues/110417)\r\n## Summary\r\nIn
the Ingest Node Pipelines section, when the users created a
new\r\npipeline selecting de Community ID processor the users could set
a\r\nnon-integer number in this field. Then, they received a server
side\r\nerror when tried to create a pipeline. For fixing this, a
validation\r\nmust be added in the client.\r\n\r\nWe didn't have a
reusable validation for this case, but we did have a\r\ncustom
validation for integer values in the Index lifecycle
management\r\nplugin. We also had the necessary translation in that
plugin. So I went\r\nforward with:\r\n\r\n* I created a new integer
validator in the `es_ui_shared` package as it\r\nis a fairly common
validation and we could take advantage of it in the\r\nfuture. Also
added a couple of unit test there for this validator.\r\n\r\n* I reused
in the `ingest_pipelines` plugin the strings that already\r\nexisted in
`index_lifecycle_management`.\r\n\r\n* I added the new validation in the
Community ID form in the\r\n`ingest_pipelines` plugin. Also added some
test verifying that the\r\nprocessor doesn't create when the seeds
validation fails.\r\n\r\n* Changed the method in the
`index_lifecycle_management` validator so\r\nnow it uses the reusable
one.\r\n\r\nNow the Ingest pipeline forms shows the validation when the
number is\r\nnot an integer:\r\n![Screenshot 2024-10-16 at 12
16\r\n47](https://github.com/user-attachments/assets/1db9ad22-b144-44a5-9012-d3ebd5a19b6f)\r\n\r\nAnd
the `index_lifecycle_management` still shows the validations
as\r\nexpected:\r\n\r\n<img width=\"756\" alt=\"Screenshot 2024-10-16 at
11 49
53\"\r\nsrc=\"https://github.com/user-attachments/assets/680886a5-355e-4637-9da9-4b93b396e751\">","sha":"5a42e5861dc53911e10a3140d8cb5366fea98ff7","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Kibana
Management","release_note:skip","v9.0.0","Feature:Ingest Node
Pipelines","backport:prev-minor","v8.17.0"],"title":"Create a common Int
Validator and use it in ingest_pipelines and
Index_lifecycle_management","number":196527,"url":"https://github.com/elastic/kibana/pull/196527","mergeCommit":{"message":"Create
a common Int Validator and use it in ingest_pipelines and
Index_lifecycle_management (#196527)\n\nCloses [#110417
](https://github.com/elastic/kibana/issues/110417)\r\n## Summary\r\nIn
the Ingest Node Pipelines section, when the users created a
new\r\npipeline selecting de Community ID processor the users could set
a\r\nnon-integer number in this field. Then, they received a server
side\r\nerror when tried to create a pipeline. For fixing this, a
validation\r\nmust be added in the client.\r\n\r\nWe didn't have a
reusable validation for this case, but we did have a\r\ncustom
validation for integer values in the Index lifecycle
management\r\nplugin. We also had the necessary translation in that
plugin. So I went\r\nforward with:\r\n\r\n* I created a new integer
validator in the `es_ui_shared` package as it\r\nis a fairly common
validation and we could take advantage of it in the\r\nfuture. Also
added a couple of unit test there for this validator.\r\n\r\n* I reused
in the `ingest_pipelines` plugin the strings that already\r\nexisted in
`index_lifecycle_management`.\r\n\r\n* I added the new validation in the
Community ID form in the\r\n`ingest_pipelines` plugin. Also added some
test verifying that the\r\nprocessor doesn't create when the seeds
validation fails.\r\n\r\n* Changed the method in the
`index_lifecycle_management` validator so\r\nnow it uses the reusable
one.\r\n\r\nNow the Ingest pipeline forms shows the validation when the
number is\r\nnot an integer:\r\n![Screenshot 2024-10-16 at 12
16\r\n47](https://github.com/user-attachments/assets/1db9ad22-b144-44a5-9012-d3ebd5a19b6f)\r\n\r\nAnd
the `index_lifecycle_management` still shows the validations
as\r\nexpected:\r\n\r\n<img width=\"756\" alt=\"Screenshot 2024-10-16 at
11 49
53\"\r\nsrc=\"https://github.com/user-attachments/assets/680886a5-355e-4637-9da9-4b93b396e751\">","sha":"5a42e5861dc53911e10a3140d8cb5366fea98ff7"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196527","number":196527,"mergeCommit":{"message":"Create
a common Int Validator and use it in ingest_pipelines and
Index_lifecycle_management (#196527)\n\nCloses [#110417
](https://github.com/elastic/kibana/issues/110417)\r\n## Summary\r\nIn
the Ingest Node Pipelines section, when the users created a
new\r\npipeline selecting de Community ID processor the users could set
a\r\nnon-integer number in this field. Then, they received a server
side\r\nerror when tried to create a pipeline. For fixing this, a
validation\r\nmust be added in the client.\r\n\r\nWe didn't have a
reusable validation for this case, but we did have a\r\ncustom
validation for integer values in the Index lifecycle
management\r\nplugin. We also had the necessary translation in that
plugin. So I went\r\nforward with:\r\n\r\n* I created a new integer
validator in the `es_ui_shared` package as it\r\nis a fairly common
validation and we could take advantage of it in the\r\nfuture. Also
added a couple of unit test there for this validator.\r\n\r\n* I reused
in the `ingest_pipelines` plugin the strings that already\r\nexisted in
`index_lifecycle_management`.\r\n\r\n* I added the new validation in the
Community ID form in the\r\n`ingest_pipelines` plugin. Also added some
test verifying that the\r\nprocessor doesn't create when the seeds
validation fails.\r\n\r\n* Changed the method in the
`index_lifecycle_management` validator so\r\nnow it uses the reusable
one.\r\n\r\nNow the Ingest pipeline forms shows the validation when the
number is\r\nnot an integer:\r\n![Screenshot 2024-10-16 at 12
16\r\n47](https://github.com/user-attachments/assets/1db9ad22-b144-44a5-9012-d3ebd5a19b6f)\r\n\r\nAnd
the `index_lifecycle_management` still shows the validations
as\r\nexpected:\r\n\r\n<img width=\"756\" alt=\"Screenshot 2024-10-16 at
11 49
53\"\r\nsrc=\"https://github.com/user-attachments/assets/680886a5-355e-4637-9da9-4b93b396e751\">","sha":"5a42e5861dc53911e10a3140d8cb5366fea98ff7"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Sonia Sanz Vivas <[email protected]>
  • Loading branch information
kibanamachine and SoniaSanzV authored Oct 24, 2024
1 parent 107344d commit b6d993f
Show file tree
Hide file tree
Showing 12 changed files with 151 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ export * from './lowercase_string';
export * from './is_json';
export * from './number_greater_than';
export * from './number_smaller_than';
export * from './is_integer';
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { ValidationFuncArg } from '../../hook_form_lib';
import { isInteger } from './is_integer';

describe('isInteger', () => {
const message = 'test error message';
const code = 'ERR_NOT_INT_NUMBER';

const validate = isInteger({ message });
const validator = (value: unknown) => validate({ value } as ValidationFuncArg<any, any>);

test('should return undefined if value is integer number', () => {
expect(validator(5)).toBeUndefined();
});

test('should return undefined if value string that can be parsed to integer', () => {
expect(validator('5')).toBeUndefined();
});

test('should return Validation function if value is not integer number', () => {
expect(validator(5.3)).toMatchObject({ message, code });
});

test('should return Validation function if value a string that can not be parsed to number but is not an integer', () => {
expect(validator('5.3')).toMatchObject({ message, code });
});

test('should return Validation function if value a string that can not be parsed to number', () => {
expect(validator('test')).toMatchObject({ message, code });
});

test('should return Validation function if value is boolean', () => {
expect(validator(false)).toMatchObject({ message, code });
});

test('should return undefined if value is empty', () => {
expect(validator('')).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { ValidationFunc } from '../../hook_form_lib';
import { ERROR_CODE } from './types';

export const isInteger =
({ message }: { message: string }) =>
(...args: Parameters<ValidationFunc>): ReturnType<ValidationFunc<any, ERROR_CODE>> => {
const [{ value }] = args;

if (
value === '' ||
(typeof value === 'number' && Number.isInteger(value)) ||
(typeof value === 'string' && Number.isInteger(Number(value)))
) {
return undefined;
}

return { message, code: 'ERR_NOT_INT_NUMBER' };
};
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ export type ERROR_CODE =
| 'ERR_LOWERCASE_STRING'
| 'ERR_JSON_FORMAT'
| 'ERR_SMALLER_THAN_NUMBER'
| 'ERR_GREATER_THAN_NUMBER';
| 'ERR_GREATER_THAN_NUMBER'
| 'ERR_NOT_INT_NUMBER';
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@ import { i18nTexts } from '../i18n_texts';
import {
ifExistsNumberGreaterThanZero,
ifExistsNumberNonNegative,
integerValidator,
minAgeGreaterThanPreviousPhase,
rolloverThresholdsValidator,
downsampleIntervalMultipleOfPreviousOne,
} from './validations';

const rolloverFormPaths = Object.values(ROLLOVER_FORM_PATHS);

const { emptyField, numberGreaterThanField } = fieldValidators;
const { emptyField, isInteger, numberGreaterThanField } = fieldValidators;

const serializers = {
stringToNumber: (v: string): any => (v != null ? parseInt(v, 10) : undefined),
Expand Down Expand Up @@ -150,7 +149,7 @@ const getMinAgeField = (phase: PhaseWithTiming, defaultValue?: string) => ({
validator: ifExistsNumberNonNegative,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
{
validator: minAgeGreaterThanPreviousPhase(phase),
Expand Down Expand Up @@ -192,7 +191,7 @@ const getDownsampleSchema = (phase: PhaseWithDownsample): FormSchema['downsample
validator: ifExistsNumberGreaterThanZero,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
{
validator: downsampleIntervalMultipleOfPreviousOne(phase),
Expand Down Expand Up @@ -381,7 +380,7 @@ export const getSchema = (isCloudEnabled: boolean): FormSchema => ({
validator: ifExistsNumberGreaterThanZero,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
],
fieldsToValidateOnChange: rolloverFormPaths,
Expand All @@ -396,7 +395,7 @@ export const getSchema = (isCloudEnabled: boolean): FormSchema => ({
validator: ifExistsNumberGreaterThanZero,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
],
serializer: serializers.stringToNumber,
Expand Down Expand Up @@ -424,7 +423,7 @@ export const getSchema = (isCloudEnabled: boolean): FormSchema => ({
validator: ifExistsNumberGreaterThanZero,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
],
serializer: serializers.stringToNumber,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,6 @@ export const rolloverThresholdsValidator: ValidationFunc = ({ form, path }) => {
}
};

export const integerValidator: ValidationFunc<FormInternal, string, string> = (arg) => {
if (!Number.isInteger(Number(arg.value ?? ''))) {
return { message: i18nTexts.editPolicy.errors.integerRequired };
}
};

export const createPolicyNameValidations = ({
policies,
isClonedPolicy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,14 @@ const configurationFormSchema: FormSchema = {
formatters: [fieldFormatters.toInt],
validations: [
{
validator: ({ value }) => {
// TODO: Replace with validator added in https://github.com/elastic/kibana/pull/196527/
if (!Number.isInteger(Number(value ?? ''))) {
return {
message: i18n.translate(
'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldIntegerError',
{
defaultMessage: 'Only integers are allowed.',
}
),
};
}
},
validator: fieldValidators.isInteger({
message: i18n.translate(
'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldIntegerError',
{
defaultMessage: 'Only integers are allowed.',
}
),
}),
},
{
validator: ({ value, formData, customData }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,52 @@ describe('Processor: Community id', () => {
seed: 10,
});
});

test('should not add a processor if the seedField is smaller than min_value', async () => {
const {
actions: { saveNewProcessor },
form,
} = testBed;

form.setInputValue('seedField.input', '-1');

// Save the field with new changes
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, COMMUNITY_ID_TYPE);

expect(processors).toHaveLength(0);
});

test('should not add a processor if the seedField is bigger than max_value', async () => {
const {
actions: { saveNewProcessor },
form,
} = testBed;

form.setInputValue('seedField.input', '65536');

// Save the field with new changes
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, COMMUNITY_ID_TYPE);

expect(processors).toHaveLength(0);
});

test('should not add a processor if the seedField is not an integer', async () => {
const {
actions: { saveNewProcessor },
form,
} = testBed;

form.setInputValue('seedField.input', '10.2');

// Save the field with new changes
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, COMMUNITY_ID_TYPE);

expect(processors).toHaveLength(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ const seedValidator = {
values: { minValue: SEED_MIN_VALUE },
}),
}),
int: fieldValidators.isInteger({
message: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError',
{
defaultMessage: 'Only integers are allowed.',
}
),
}),
};

const fieldsConfig: FieldsConfig = {
Expand Down Expand Up @@ -183,7 +191,7 @@ const fieldsConfig: FieldsConfig = {
{
validator: (field) => {
if (field.value) {
return seedValidator.max(field) ?? seedValidator.min(field);
return seedValidator.max(field) ?? seedValidator.min(field) ?? seedValidator.int(field);
}
},
},
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/translations/translations/fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -24131,6 +24131,7 @@
"xpack.ingestPipelines.pipelineEditor.communityId.icmpCodeLabel": "Code ICMP (facultatif)",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeHelpText": "Champ contenant le type ICMP de la destination. La valeur par défaut est {defaultValue}.",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeLabel": "Type ICMP (facultatif)",
"xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError": "Seuls les entiers sont autorisés.",
"xpack.ingestPipelines.pipelineEditor.communityId.seedHelpText": "Valeur initiale du hachage de l'ID de communauté. La valeur par défaut est {defaultValue}.",
"xpack.ingestPipelines.pipelineEditor.communityId.seedLabel": "Valeur initiale (facultatif)",
"xpack.ingestPipelines.pipelineEditor.communityId.seedMaxNumberError": "Ce nombre doit être inférieur ou égal à {maxValue}.",
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -23878,6 +23878,7 @@
"xpack.ingestPipelines.pipelineEditor.communityId.icmpCodeLabel": "ICMPコード(任意)",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeHelpText": "デスティネーションICMPタイプを含むフィールド。デフォルトは{defaultValue}です。",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeLabel": "ICMPタイプ(任意)",
"xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError": "整数のみを使用できます。",
"xpack.ingestPipelines.pipelineEditor.communityId.seedHelpText": "コミュニティIDハッシュのシード。デフォルトは{defaultValue}です。",
"xpack.ingestPipelines.pipelineEditor.communityId.seedLabel": "シード(任意)",
"xpack.ingestPipelines.pipelineEditor.communityId.seedMaxNumberError": "この数は{maxValue}以下でなければなりません。",
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -23912,6 +23912,7 @@
"xpack.ingestPipelines.pipelineEditor.communityId.icmpCodeLabel": "ICMP 代码(可选)",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeHelpText": "包含目标 ICMP 类型的字段。默认为 {defaultValue}。",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeLabel": "ICMP 类型(可选)",
"xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError": "仅允许使用整数。",
"xpack.ingestPipelines.pipelineEditor.communityId.seedHelpText": "社区 ID 哈希的种子。默认为 {defaultValue}。",
"xpack.ingestPipelines.pipelineEditor.communityId.seedLabel": "种子(可选)",
"xpack.ingestPipelines.pipelineEditor.communityId.seedMaxNumberError": "此数字必须等于或小于 {maxValue}。",
Expand Down

0 comments on commit b6d993f

Please sign in to comment.