From 9eb53bb39990b144052f3f834fa0803568511197 Mon Sep 17 00:00:00 2001 From: Gary Sassano <10464497+garysassano@users.noreply.github.com> Date: Fri, 29 Mar 2024 19:12:48 +0100 Subject: [PATCH] fix(sdk): add cron validation for `cloud.Schedule` (#6090) Closes #5985 ## Checklist - [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted) - [x] Description explains motivation and solution - [x] Tests added (always) - [ ] Docs updated (only required for features) - [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing *By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*. --- examples/tests/sdk_tests/schedule/init.test.w | 16 ++++++++++++---- .../tests/sdk_tests/schedule/on_tick.test.w | 2 +- libs/awscdk/test/schedule.test.ts | 8 ++++---- libs/wingsdk/.projen/deps.json | 4 ++++ libs/wingsdk/.projenrc.ts | 1 + libs/wingsdk/package.json | 2 ++ libs/wingsdk/src/cloud/schedule.ts | 17 +++++++++++++---- .../wingsdk/test/target-tf-aws/schedule.test.ts | 4 +--- pnpm-lock.yaml | 13 ++++++++++--- 9 files changed, 48 insertions(+), 19 deletions(-) diff --git a/examples/tests/sdk_tests/schedule/init.test.w b/examples/tests/sdk_tests/schedule/init.test.w index dc8c3bd27a0..ee6cdcbda87 100644 --- a/examples/tests/sdk_tests/schedule/init.test.w +++ b/examples/tests/sdk_tests/schedule/init.test.w @@ -4,6 +4,7 @@ skip: true bring cloud; bring util; +bring expect; new cloud.Schedule( rate: 5m ) as "s0"; @@ -16,14 +17,14 @@ if (util.env("WING_TARGET") != "sim") { } catch e { error = e; } - assert(error == "rate or cron need to be filled."); + expect.equal(error, "rate or cron need to be filled."); try { new cloud.Schedule( rate: 2m, cron: "* * * * *" ) as "s2"; } catch e { error = e; } - assert(error == "rate and cron cannot be configured simultaneously."); + expect.equal(error, "rate and cron cannot be configured simultaneously."); try { @@ -31,12 +32,19 @@ if (util.env("WING_TARGET") != "sim") { } catch e { error = e; } - assert(error == "rate can not be set to less than 1 minute."); + expect.equal(error, "rate can not be set to less than 1 minute."); try { new cloud.Schedule( cron: "* * * * * *" ) as "s4"; } catch e { error = e; } - assert(error == "cron string must be UNIX cron format [minute] [hour] [day of month] [month] [day of week]"); + expect.equal(error, "cron string must be in UNIX cron format"); + + try { + new cloud.Schedule( cron: "* * * * ?" ) as "s5"; + } catch e { + error = e; + } + expect.equal(error, "cron string must be in UNIX cron format"); } \ No newline at end of file diff --git a/examples/tests/sdk_tests/schedule/on_tick.test.w b/examples/tests/sdk_tests/schedule/on_tick.test.w index 4b19dcc9916..2c2ec9df0fc 100644 --- a/examples/tests/sdk_tests/schedule/on_tick.test.w +++ b/examples/tests/sdk_tests/schedule/on_tick.test.w @@ -2,7 +2,7 @@ bring cloud; bring util; // every minute -let from_cron = new cloud.Schedule( cron: "* * * * ?" ) as "from_cron"; +let from_cron = new cloud.Schedule( cron: "* * * * *" ) as "from_cron"; let from_rate = new cloud.Schedule( rate: 1m ) as "from_rate"; let c1 = new cloud.Counter() as "c1"; let c2 = new cloud.Counter() as "c2"; diff --git a/libs/awscdk/test/schedule.test.ts b/libs/awscdk/test/schedule.test.ts index 453ae1712cd..22c099f3868 100644 --- a/libs/awscdk/test/schedule.test.ts +++ b/libs/awscdk/test/schedule.test.ts @@ -158,9 +158,7 @@ test("cron with more than five values", () => { new cloud.Schedule(app, "Schedule", { cron: "0/1 * ? * * *", }) - ).toThrow( - "cron string must be UNIX cron format [minute] [hour] [day of month] [month] [day of week]" - ); + ).toThrow("cron string must be in UNIX cron format"); }); test("schedule without rate or cron", () => { @@ -196,5 +194,7 @@ test("cron with day of month and day of week configured at the same time", () => new cloud.Schedule(app, "Schedule", { cron: "* * 1 * 1", }) - ).toThrow("Cannot restrict both 'day-of-month' and 'day-of-week' in a cron expression, at least one must be '*'"); + ).toThrow( + "Cannot restrict both 'day-of-month' and 'day-of-week' in a cron expression, at least one must be '*'" + ); }); diff --git a/libs/wingsdk/.projen/deps.json b/libs/wingsdk/.projen/deps.json index a3d550c41d8..9eac9925809 100644 --- a/libs/wingsdk/.projen/deps.json +++ b/libs/wingsdk/.projen/deps.json @@ -275,6 +275,10 @@ "name": "cron-parser", "type": "bundled" }, + { + "name": "cron-validator", + "type": "bundled" + }, { "name": "express", "type": "bundled" diff --git a/libs/wingsdk/.projenrc.ts b/libs/wingsdk/.projenrc.ts index f2abc59a017..c3a945b332d 100644 --- a/libs/wingsdk/.projenrc.ts +++ b/libs/wingsdk/.projenrc.ts @@ -82,6 +82,7 @@ const project = new cdk.JsiiProject({ "ioredis", "jsonschema", "ajv", + "cron-validator", // fs module dependency "yaml", "toml", diff --git a/libs/wingsdk/package.json b/libs/wingsdk/package.json index 9e6cc45117e..974459351a6 100644 --- a/libs/wingsdk/package.json +++ b/libs/wingsdk/package.json @@ -102,6 +102,7 @@ "cdktf": "0.20.3", "constructs": "^10.3", "cron-parser": "^4.9.0", + "cron-validator": "^1.3.1", "express": "^4.18.2", "glob": "^8.1.0", "google-auth-library": "^8.9.0", @@ -143,6 +144,7 @@ "ajv", "cdktf", "cron-parser", + "cron-validator", "express", "glob", "google-auth-library", diff --git a/libs/wingsdk/src/cloud/schedule.ts b/libs/wingsdk/src/cloud/schedule.ts index a452793402e..bd9e12094e1 100644 --- a/libs/wingsdk/src/cloud/schedule.ts +++ b/libs/wingsdk/src/cloud/schedule.ts @@ -1,4 +1,5 @@ import { Construct } from "constructs"; +import { isValidCron } from "cron-validator"; import { Function, FunctionProps } from "./function"; import { fqnForType } from "../constants"; import { AbstractMemberError } from "../core/errors"; @@ -70,10 +71,18 @@ export class Schedule extends Resource { if (rate && rate.seconds < 60) { throw new Error("rate can not be set to less than 1 minute."); } - if (cron && cron.split(" ").length > 5) { - throw new Error( - "cron string must be UNIX cron format [minute] [hour] [day of month] [month] [day of week]" - ); + // Check for valid UNIX cron format + // https://www.ibm.com/docs/en/db2/11.5?topic=task-unix-cron-format + if ( + cron && + !isValidCron(cron, { + alias: true, + allowSevenAsSunday: true, + allowBlankDay: false, + seconds: false, + }) + ) { + throw new Error("cron string must be in UNIX cron format"); } } diff --git a/libs/wingsdk/test/target-tf-aws/schedule.test.ts b/libs/wingsdk/test/target-tf-aws/schedule.test.ts index 00e867c7534..6c004260607 100644 --- a/libs/wingsdk/test/target-tf-aws/schedule.test.ts +++ b/libs/wingsdk/test/target-tf-aws/schedule.test.ts @@ -241,9 +241,7 @@ test("cron with more than five values", () => { new cloud.Schedule(app, "Schedule", { cron: "0/1 * * * * *", }) - ).toThrow( - "cron string must be UNIX cron format [minute] [hour] [day of month] [month] [day of week]" - ); + ).toThrow("cron string must be in UNIX cron format"); }); test("schedule without rate or cron", () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a812a770651..a36dda7f447 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1326,6 +1326,9 @@ importers: cron-parser: specifier: ^4.9.0 version: 4.9.0 + cron-validator: + specifier: ^1.3.1 + version: 1.3.1 express: specifier: ^4.18.2 version: 4.18.2 @@ -13694,6 +13697,10 @@ packages: luxon: 3.4.3 dev: false + /cron-validator@1.3.1: + resolution: {integrity: sha512-C1HsxuPCY/5opR55G5/WNzyEGDWFVG+6GLrA+fW/sCTcP6A6NTjUP2AK7B8n2PyFs90kDG2qzwm8LMheADku6A==} + dev: false + /cross-fetch@3.1.8: resolution: {integrity: sha512-cvA+JwZoU0Xq+h6WkMvAUqPEYy92Obet6UdKLfW60qn99ftItKjB5T+BkyWOFWe2pUyfQ+IJHmpOTznqk1M6Kg==} dependencies: @@ -14187,7 +14194,7 @@ packages: dependencies: semver: 7.5.4 shelljs: 0.8.5 - typescript: 5.5.0-dev.20240325 + typescript: 5.5.0-dev.20240328 dev: true /dset@3.1.2: @@ -22795,8 +22802,8 @@ packages: engines: {node: '>=14.17'} hasBin: true - /typescript@5.5.0-dev.20240325: - resolution: {integrity: sha512-3kSBj+PWimbJvH3s/sQiIU7zBIOWAEpHdD3lykIykePugc1qXNIK8d5/WJJNpzH4q2n6gY8pz8lLoe/HanVGnA==} + /typescript@5.5.0-dev.20240328: + resolution: {integrity: sha512-psmUop0GPegCmqZqysToBqpX/pKW/0YGwVc/Ma0kDOkruXO40euXu2GL5UM6KbL+nnyHiMtWiTmVQ+7To9s9yQ==} engines: {node: '>=14.17'} hasBin: true dev: true