From 78d3e1ef2a87334f691d54db28e849accc585286 Mon Sep 17 00:00:00 2001 From: Giuseppe Leo Date: Thu, 30 Jan 2025 18:49:43 +0100 Subject: [PATCH] Crontab @daily marked as invalid in Rancher Cronjobs (#13214) * Export abstracted chron rule validation and message to match both current architectures * Add chronSchedule to the parameterized tests; Add @daily case * Add missing JSDocs for comprension * Update Cronstrue to 2.53 * Remove unnecessary import * Fix lint issues * Add sublabel and chron type with hints * Add @daily case to the input stories * Add multiline type story for input * Added Cron error case * Add chron tests for input label * Add periodical case as exceptions * Linter fix * Correct unit test definition --- package.json | 2 +- pkg/rancher-components/package.json | 2 +- .../Form/LabeledInput/LabeledInput.test.ts | 19 ++++++- .../Form/LabeledInput/LabeledInput.vue | 17 ++++++- shell/package.json | 2 +- shell/utils/validators/cron-schedule.js | 9 +++- .../formRules/__tests__/index.test.ts | 31 ++++++------ shell/utils/validators/formRules/index.ts | 22 ++++++-- storybook/src/stories/Form/Input.mdx | 20 ++++++++ storybook/src/stories/Form/Input.stories.ts | 50 +++++++++++++++++++ yarn.lock | 13 ++--- 11 files changed, 149 insertions(+), 38 deletions(-) diff --git a/package.json b/package.json index 24f45bdc8a2..77853c2c48e 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,7 @@ "cookie": "0.7.0", "cookie-universal": "2.2.2", "cron-validator": "1.2.0", - "cronstrue": "1.95.0", + "cronstrue": "2.53.0", "cross-env": "7.0.3", "custom-event-polyfill": "1.0.7", "d3": "7.3.0", diff --git a/pkg/rancher-components/package.json b/pkg/rancher-components/package.json index d02cf332f37..f5ad2078c5b 100644 --- a/pkg/rancher-components/package.json +++ b/pkg/rancher-components/package.json @@ -36,7 +36,7 @@ "babel-eslint": "10.1.0", "core-js": "3.40.0", "cron-validator": "1.3.1", - "cronstrue": "2.50.0", + "cronstrue": "2.53.0", "eslint-plugin-import": "2.31.0", "eslint-plugin-node": "11.1.0", "eslint-plugin-promise": "5.2.0", diff --git a/pkg/rancher-components/src/components/Form/LabeledInput/LabeledInput.test.ts b/pkg/rancher-components/src/components/Form/LabeledInput/LabeledInput.test.ts index 4846fdaa7e4..e3e48943cb9 100644 --- a/pkg/rancher-components/src/components/Form/LabeledInput/LabeledInput.test.ts +++ b/pkg/rancher-components/src/components/Form/LabeledInput/LabeledInput.test.ts @@ -20,7 +20,7 @@ describe('component: LabeledInput', () => { expect(wrapper.emitted('update:value')![0][0]).toBe(value); }); - it('using mode "multiline" should emit input value correctly', () => { + it('using type "multiline" should emit input value correctly', () => { const value = 'any-string'; const delay = 1; const wrapper = mount(LabeledInput, { @@ -37,4 +37,21 @@ describe('component: LabeledInput', () => { expect(wrapper.emitted('update:value')).toHaveLength(1); expect(wrapper.emitted('update:value')![0][0]).toBe(value); }); + + describe('using type "chron"', () => { + it.each([ + ['0 * * * *', 'Every hour, every day'], + ['@daily', 'At 12:00 AM, every day'], + ['You must fail! Go!', '%generic.invalidCron%'], + ])('passing value %p should display hint %p', (value, hint) => { + const wrapper = mount(LabeledInput, { + propsData: { value, type: 'cron' }, + mocks: { $store: { getters: { 'i18n/t': jest.fn() } } } + }); + + const subLabel = wrapper.find('[data-testid="sub-label"]'); + + expect(subLabel.text()).toBe(hint); + }); + }); }); diff --git a/pkg/rancher-components/src/components/Form/LabeledInput/LabeledInput.vue b/pkg/rancher-components/src/components/Form/LabeledInput/LabeledInput.vue index 8c03ee1340c..a05a648dcf8 100644 --- a/pkg/rancher-components/src/components/Form/LabeledInput/LabeledInput.vue +++ b/pkg/rancher-components/src/components/Form/LabeledInput/LabeledInput.vue @@ -179,14 +179,28 @@ export default defineComponent({ if (this.type !== 'cron' || !this.value) { return; } + + // TODO - #13202: This is required due use of 2 libraries and 3 different libraries through the code. + const predefined = [ + '@yearly', + '@annually', + '@monthly', + '@weekly', + '@daily', + '@midnight', + '@hourly' + ]; + const isPredefined = predefined.includes(this.value as string); + // refer https://github.com/GuillaumeRochat/cron-validator#readme - if (!isValidCron(this.value as string, { + if (!isPredefined && !isValidCron(this.value as string, { alias: true, allowBlankDay: true, allowSevenAsSunday: true, })) { return this.t('generic.invalidCron'); } + try { const hint = cronstrue.toString(this.value as string || '', { verbose: true }); @@ -382,6 +396,7 @@ export default defineComponent({
cronstrue.toString(text, { verbose: true }), + message: 'validation.invalidCron' +}; diff --git a/shell/utils/validators/formRules/__tests__/index.test.ts b/shell/utils/validators/formRules/__tests__/index.test.ts index d021a36091a..679b58ffe37 100644 --- a/shell/utils/validators/formRules/__tests__/index.test.ts +++ b/shell/utils/validators/formRules/__tests__/index.test.ts @@ -27,22 +27,6 @@ describe('formRules', () => { expect(formRuleResult).toStrictEqual(expectedResult); }); - it('"cronSchedule" : returns undefined when valid cron string value supplied', () => { - const testValue = '0 * * * *'; - const formRuleResult = formRules.cronSchedule(testValue); - - expect(formRuleResult).toBeUndefined(); - }); - - it('"cronSchedule" : returns the correct message when invalid cron string value supplied', () => { - // specific logic of what constitutes a cron string is in the "cronstrue" function in an external library and not tested here - const testValue = '0 * * **'; - const formRuleResult = formRules.cronSchedule(testValue); - const expectedResult = JSON.stringify({ message: 'validation.invalidCron' }); - - expect(formRuleResult).toStrictEqual(expectedResult); - }); - it('"https" : returns undefined when valid https url value is supplied', () => { const testValue = 'https://url.com'; const formRuleResult = formRules.https(testValue); @@ -1112,6 +1096,13 @@ describe('formRules', () => { expect(formRuleResult).toStrictEqual(expectedResult); }); + /** + * Test all factory validators + * @param rule - the name of the factory validator + * @param argument - the value to validate + * @param correctValues - an array of values that should pass the validation + * @param wrongValues - an array of values that should fail the validation + */ describe.each([ ['minValue', 2, [3], [1]], ['maxValue', 256, [1], [300]], @@ -1133,12 +1124,18 @@ describe('formRules', () => { }); }); + /** + * Test all standard validators + * @param rule - the name of the standard validator + * @param correctValues - an array of values that should pass the validation + * @param wrongValues - an array of values that should fail the validation + */ describe.each([ ['requiredInt', [2, 2.2], ['e']], ['isInteger', ['2', 2, 0], [2.2, 'e', '1.0']], ['isPositive', ['0', 1], [-1]], ['isOctal', ['0', 0, 10], ['01']], - + ['cronSchedule', ['0 * * * *', '@daily'], ['0 * * **']], ])('given validator %p', (rule, correctValues, wrongValues) => { it.each(wrongValues as [])('should return error for value %p', (wrong) => { const formRuleResult = (formRules as any)[rule](wrong); diff --git a/shell/utils/validators/formRules/index.ts b/shell/utils/validators/formRules/index.ts index 3df2d019632..ab7de7c1175 100644 --- a/shell/utils/validators/formRules/index.ts +++ b/shell/utils/validators/formRules/index.ts @@ -4,14 +4,26 @@ import isEmpty from 'lodash/isEmpty'; import has from 'lodash/has'; import isUrl from 'is-url'; // import uniq from 'lodash/uniq'; -import cronstrue from 'cronstrue'; import { Translation } from '@shell/types/t'; import { isHttps, isLocalhost, hasTrailingForwardSlash } from '@shell/utils/validators/setting'; +import { cronScheduleRule } from '@shell/utils/validators/cron-schedule'; // import uniq from 'lodash/uniq'; -export type Validator = (val: any, arg?: any) => T; -export type ValidatorFactory = (arg1: any, arg2?: any) => Validator +/** + * Fixed validation rule which require only the value to be evaluated + * @param value + * @returns { string | undefined } + */ +export type Validator = (value: any, arg?: any) => T; + +/** + * Factory function which returns a validation rule + * @param arg Argument used as part of the validation rule process, not necessarily as parameter of the validation rule + * @param value Value to be evaluated + * @returns { Validator } + */ +export type ValidatorFactory = (arg: any, value?: any) => Validator type ServicePort = { name?: string, @@ -131,9 +143,9 @@ export default function(t: Translation, { key = 'Value' }: ValidationOptions): { const cronSchedule: Validator = (val: string) => { try { - cronstrue.toString(val, { verbose: true }); + cronScheduleRule.validation(val); } catch (e) { - return t('validation.invalidCron'); + return t(cronScheduleRule.message); } }; diff --git a/storybook/src/stories/Form/Input.mdx b/storybook/src/stories/Form/Input.mdx index 11b980008e7..a27fa6b95ac 100644 --- a/storybook/src/stories/Form/Input.mdx +++ b/storybook/src/stories/Form/Input.mdx @@ -34,3 +34,23 @@ Input elements with type ‘text’ allow users to enter any combination of lett #### Error + +#### SubLabel + +Addition information can be added to the input field using the `subLabel` prop. + + + +#### Cron Type + +The `cron` prop can be used to add [Cron language](https://en.wikipedia.org/wiki/Cron) hints. + + + + + + + +#### Multiline Type + + diff --git a/storybook/src/stories/Form/Input.stories.ts b/storybook/src/stories/Form/Input.stories.ts index f7adf847784..9502cb16135 100644 --- a/storybook/src/stories/Form/Input.stories.ts +++ b/storybook/src/stories/Form/Input.stories.ts @@ -64,3 +64,53 @@ export const Error: Story = { tooltipKey: 'Error message' }, }; + +export const SubLabel: Story = { + ...Default, + args: { + label: 'Name', + subLabel: 'Additional information', + type: 'text', + value: 'Simon', + }, +}; + +export const CronType: Story = { + ...Default, + args: { + label: 'Period', + type: 'cron', + value: '0 * * * *', + }, +}; + +export const CronTypeDaily: Story = { + ...Default, + args: { + label: 'Period', + type: 'cron', + value: '@daily', + }, +}; + +export const CronTypeError: Story = { + ...Default, + args: { + label: 'Period', + type: 'cron', + value: 'not a cron expression', + }, +}; + +export const MultilineType: Story = { + ...Default, + args: { + label: 'Period', + type: 'multiline', + value: `this +is +a +multiline +text`, + }, +}; diff --git a/yarn.lock b/yarn.lock index 77e90d2dc72..ccdf232fe21 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6386,15 +6386,10 @@ cron-validator@1.3.1: resolved "https://registry.yarnpkg.com/cron-validator/-/cron-validator-1.3.1.tgz#8f2fe430f92140df77f91178ae31fc1e3a48a20e" integrity sha512-C1HsxuPCY/5opR55G5/WNzyEGDWFVG+6GLrA+fW/sCTcP6A6NTjUP2AK7B8n2PyFs90kDG2qzwm8LMheADku6A== -cronstrue@1.95.0: - version "1.95.0" - resolved "https://registry.npmjs.org/cronstrue/-/cronstrue-1.95.0.tgz#171df1fad8b0f0cb636354dd1d7842161c15478f" - integrity sha512-CdbQ17Z8Na2IdrK1SiD3zmXfE66KerQZ8/iApkGsxjmUVGJPS9M9oK4FZC3LM6ohUjjq3UeaSk+90Cf3QbXDfw== - -cronstrue@2.50.0: - version "2.50.0" - resolved "https://registry.yarnpkg.com/cronstrue/-/cronstrue-2.50.0.tgz#eabba0f915f186765258b707b7a3950c663b5573" - integrity sha512-ULYhWIonJzlScCCQrPUG5uMXzXxSixty4djud9SS37DoNxDdkeRocxzHuAo4ImRBUK+mAuU5X9TSwEDccnnuPg== +cronstrue@2.53.0: + version "2.53.0" + resolved "https://registry.npmjs.org/cronstrue/-/cronstrue-2.53.0.tgz#5bbcd7483636b99379480f624faef5056f3efbd8" + integrity sha512-CkAcaI94xL8h6N7cGxgXfR5D7oV2yVtDzB9vMZP8tIgPyEv/oc/7nq9rlk7LMxvc3N+q6LKZmNLCVxJRpyEg8A== cross-env@7.0.3: version "7.0.3"