Skip to content

Commit

Permalink
Crontab @daily marked as invalid in Rancher Cronjobs (#13214)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
cnotv authored Jan 30, 2025
1 parent 2d5a458 commit 78d3e1e
Show file tree
Hide file tree
Showing 11 changed files with 149 additions and 38 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion pkg/rancher-components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand All @@ -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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down Expand Up @@ -382,6 +396,7 @@ export default defineComponent({
<div
v-if="cronHint || subLabel"
class="sub-label"
data-testid="sub-label"
>
<div
v-if="cronHint"
Expand Down
2 changes: 1 addition & 1 deletion shell/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"cookie": "0.7.0",
"core-js": "3.40.0",
"cron-validator": "1.3.1",
"cronstrue": "2.50.0",
"cronstrue": "2.53.0",
"cross-env": "7.0.3",
"css-loader": "6.7.3",
"csv-loader": "3.0.3",
Expand Down
9 changes: 7 additions & 2 deletions shell/utils/validators/cron-schedule.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@ import cronstrue from 'cronstrue';

export function cronSchedule(schedule = '', getters, errors) {
try {
cronstrue.toString(schedule, { verbose: true });
cronScheduleRule.validation(schedule);
} catch (e) {
errors.push(getters['i18n/t']('validation.invalidCron'));
errors.push(getters['i18n/t'](cronScheduleRule.message));
}
}

export const cronScheduleRule = {
validation: (text) => cronstrue.toString(text, { verbose: true }),
message: 'validation.invalidCron'
};
31 changes: 14 additions & 17 deletions shell/utils/validators/formRules/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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]],
Expand All @@ -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);
Expand Down
22 changes: 17 additions & 5 deletions shell/utils/validators/formRules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T = undefined | string> = (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<T = undefined | string> = (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,
Expand Down Expand Up @@ -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);
}
};

Expand Down
20 changes: 20 additions & 0 deletions storybook/src/stories/Form/Input.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,23 @@ Input elements with type ‘text’ allow users to enter any combination of lett
#### Error

<Canvas of={LabeledInput.Error} />

#### SubLabel

Addition information can be added to the input field using the `subLabel` prop.

<Canvas of={LabeledInput.SubLabel} />

#### Cron Type

The `cron` prop can be used to add [Cron language](https://en.wikipedia.org/wiki/Cron) hints.

<Canvas of={LabeledInput.CronType} />

<Canvas of={LabeledInput.CronTypeDaily} />

<Canvas of={LabeledInput.CronTypeError} />

#### Multiline Type

<Canvas of={LabeledInput.MultilineType} />
50 changes: 50 additions & 0 deletions storybook/src/stories/Form/Input.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
},
};
13 changes: 4 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6386,15 +6386,10 @@ [email protected]:
resolved "https://registry.yarnpkg.com/cron-validator/-/cron-validator-1.3.1.tgz#8f2fe430f92140df77f91178ae31fc1e3a48a20e"
integrity sha512-C1HsxuPCY/5opR55G5/WNzyEGDWFVG+6GLrA+fW/sCTcP6A6NTjUP2AK7B8n2PyFs90kDG2qzwm8LMheADku6A==

[email protected]:
version "1.95.0"
resolved "https://registry.npmjs.org/cronstrue/-/cronstrue-1.95.0.tgz#171df1fad8b0f0cb636354dd1d7842161c15478f"
integrity sha512-CdbQ17Z8Na2IdrK1SiD3zmXfE66KerQZ8/iApkGsxjmUVGJPS9M9oK4FZC3LM6ohUjjq3UeaSk+90Cf3QbXDfw==

[email protected]:
version "2.50.0"
resolved "https://registry.yarnpkg.com/cronstrue/-/cronstrue-2.50.0.tgz#eabba0f915f186765258b707b7a3950c663b5573"
integrity sha512-ULYhWIonJzlScCCQrPUG5uMXzXxSixty4djud9SS37DoNxDdkeRocxzHuAo4ImRBUK+mAuU5X9TSwEDccnnuPg==
[email protected]:
version "2.53.0"
resolved "https://registry.npmjs.org/cronstrue/-/cronstrue-2.53.0.tgz#5bbcd7483636b99379480f624faef5056f3efbd8"
integrity sha512-CkAcaI94xL8h6N7cGxgXfR5D7oV2yVtDzB9vMZP8tIgPyEv/oc/7nq9rlk7LMxvc3N+q6LKZmNLCVxJRpyEg8A==

[email protected]:
version "7.0.3"
Expand Down

0 comments on commit 78d3e1e

Please sign in to comment.