PR 19766 review, round two

rancher/dashboard PR 19766 (codyrancher), re-requested after e11c386, reviewed 2026-11-04. Round one is here, though its preview links now show the new code.

Preview: rancherpr19766r.mf-created-dashboard.ui.rancher.space, built from e11c386, backend mf Rancher (v2.16.0-7758e3e-head). Baseline: the shared dashboard-master preview, rebuilt today from c13add8ed, the current tip.

Verdict: approve

Both of your comments are fixed. Day of week 7 blocks, @every and timezone prefixes save, and TZ=UTC on its own stays blocked. Create, Amazon EC2, Edit and imported all give the same answers.
Cody went further than you asked, replacing cron-validator in this path with a hand-written copy of the parser K3s and RKE2 use, and it holds up. Against the real parser on 1,731,642 schedules, no crash gets through, and the only difference a person could type is a wrong timezone name, which the code comment calls out.
The list is now 23 right, 1 wrong, 5 debatable, out of 29 rows. Master is 11, 13 and 5.
Your call (card 1): that copy is 114 lines plus 119 of tests, and my round-one fix was about 15. The table in your review looks like the reason. I'd accept it anyway.
One optional nit (card 2): a 3-line branch that can't change any answer.
CI is all green. Nothing is posted, and suggested text for you is at the bottom. Your two threads are still open, and Cody has answered both.

Test it yourself

The login page defaults to OpenLDAP, so click Log in as a local user first, then use admin and RANCHER_PASSWORD.

Create: open PR: Create Custom, etcd and type any name. Set Cron Schedule to 0 0 * * 7: Create disables, and the icon's tooltip gives the 0-6 range. Then try @every 6h and TZ=UTC 0 0 * * * (Create enabled, hint says Invalid) and TZ=UTC (disabled).
Stored schedules: open PR: Edit, @every 6h. Save is now enabled on load, and it was disabled in round one. PR: Edit, invalid schedule is still disabled with the etcd tab flagged.
Imported: open PR: Imported test-k3s and try the same values under Day 2 Operations. Don't click Save here: test-k3s is a real cluster entry, and my runs intercepted that write.

Your review, Cody's answers, and what I found

Your comment	Cody's answer	Checked on the new build
`allowSevenAsSunday` lets `0 0 * * 7` through, and K3s and RKE2 then fail silently. The hint even says "only on Sunday".	"day of week is 0-6 now, so `7`, `1-7`, `5-7` and `0,7` block Create, and the error gives the range (0-6, Sunday is 0). The grey hint still reads 'only on Sunday'."	True. All four block on Create, Edit and imported, and `0 0 * * 7` on Amazon EC2. The tooltip ends "day of week (0-6, Sunday is 0)", and the etcd tab is flagged. The hint does still read "only on Sunday".
Accept `@every` and `CRON_TZ=`, or add a release note. A timezone prefix has to require a space, because `TZ=UTC` alone crashes the server.	"`@every <duration>` and `CRON_TZ=`/`TZ=` save (the prefix needs a space after it, so a bare `TZ=UTC` stays blocked), and every row of your table is a test, including the odd forms robfig parses."	True. They save and the payload is unchanged. `TZ=UTC`, `CRON_TZ=UTC` and the tab form stay blocked. 28 of the 29 rows have a test. The one without is the unknown timezone, which saves.
Cody's own note on the hint	"The grey hint under the field still flags these as it does on master, since it is `LabeledInput`'s own check."	True. The hint text is identical on master and the PR for all 105 schedules. See card 3 for what that looks like.

Create Custom with 0 0 * * 7: Create disabled, tooltip gives the 0-6 range, hint says only on Sunday — `0 0 * * 7`: Create disabled, and the tooltip gives the range. The hint still says Sunday.

Edit form for a cluster storing @every 6h: Save enabled, hint says Invalid cron schedule — Stored `@every 6h`: Save enabled, disabled in round one. The hint says Invalid, as on master.

Imported cluster with 0 0 * * 7: error icons on the field, sections and Jump to, Save disabled — Imported, `0 0 * * 7`: icons on the field, the sections and Jump to, with Save disabled.

What you can write, now

	Right	Wrong	Debatable
master	11	13	5
Round one of this PR	18	6	5
The fix I suggested in round one	19	5	5
Now, at `e11c386`	23	1	5

Green means the form does the right thing for the cluster, red means it doesn't, and uncoloured cells are debatable. Saves means Create or Save stays enabled. The small text under "Now" is the grey hint, which is the same on master.

What you write	On the cluster	master	Round one405c0e2	Nowe11c386
Accepted everywhere
Five numeric fields `0 /5 * 0 /12 * * ** * * * 0 0 * 00 0 * * 6`	Snapshots runK3s and RKE2	✅ Saves	✅ Saves	✅ Saveshint: reads fine
Steps, ranges and lists `/5 * * 0/5 * * 5/15 * * 1-5/2 * * 1,2,3 * * */60 * * 0 0 * */7`	Snapshots runK3s for some, parser for the rest	✅ Saves	✅ Saves	✅ Saveshint: reads fine
Day and month names, any case `0 0 * * MON0 0 * * mon0 0 * * Mon0 0 * * MON-FRI0 0 * * mon-fri0 0 * * MON,WED,FRI0 0 * * SUN0 0 * JAN 0 0 jan 0 0 JAN-MAR 0 0 1 JAN,JUN `	Snapshots runK3s for some, parser for the rest	✅ Saves	✅ Saves	✅ Saveshint: reads fine
Shortcuts `@yearly@annually@monthly@weekly@daily@midnight@hourly` `@daily`, `@midnight` and `@hourly` fired once each at a real local midnight. `@weekly`, `@monthly`, `@yearly` and `@annually` can't fire in a short test. In the parser they match their numeric forms for 20 years.	Snapshots runK3s and RKE2 for some, parser for the rest	✅ Saves	✅ Saves	✅ Saveshint: reads fine
`?` in one day field `0 0 ? * MON0 0 1 * ?`	Snapshots runK3s for some, parser for the rest	✅ Saves	✅ Saves	✅ Saveshint: reads fine
Leading zeros and extra spaces `00 00 * * 007 * * ` `0 0 * 0 0 * ` `0 0 * *`	Snapshots runK3s for some, parser for the rest	✅ Saves	✅ Saves	✅ Saveshint: reads fine
A date that never comes (Feb 31) `0 0 31 2 *` Blocking it would need a calendar check. The server runs normally and just never takes a snapshot.	Parses, never firesParser; K3s stays up	➖ Saves	➖ Saves	➖ Saveshint: reads fine
Empty field `(empty)` Rancher sends nothing, so the cluster keeps its default schedule.	Keeps its defaultRancher source	➖ Saves	➖ Saves	➖ Saveshint: no complaint
Where master, round one and this round differ
Day of week 7, alone or in a range `0 0 * * 70 0 * * 1-70 0 * * 5-70 0 * * 0,7` Your first comment, fixed. The grey hint still describes it as Sunday.	No snapshots, no errorK3s and RKE2	❌ Saves	❌ Saves	✅ Blockshint: reads fine
`@every` with a duration `@every 6h@every 1h30m` Your second comment, fixed. The grey hint still says Invalid, as on master.	Snapshots runK3s and RKE2	✅ Saves	❌ Blocks	✅ Saveshint: Invalid
Timezone prefix with UTC `CRON_TZ=UTC 0 0 * * TZ=UTC 0 0 * *CRON_TZ=UTC @dailyTZ=UTC @every 6h` Your second comment, fixed. UTC works with or without timezone data on the node.	Snapshots runK3s and RKE2	✅ Saves	❌ Blocks	✅ Saveshint: Invalid
Timezone prefix with a named zone `CRON_TZ=Asia/Tokyo 0 6 * * *` With timezone data on the node it fires, at the right local time. Without it there are no snapshots and no error, on both K3s and RKE2. The form can't see the node.	Runs only if the node has timezone dataK3s and RKE2	➖ Saves	➖ Blocks	➖ Saveshint: Invalid
Unknown timezone name `TZ=Nope 0 0 * * *` The one wrong row left. The code comment says the zone isn't checked, for the reason in the row above. On K3s: node Ready, 0 snapshots, no error.	No snapshots, no errorK3s	❌ Saves	✅ Blocks	❌ Saveshint: Invalid
`@every` too long to parse `@every 999999999h` Now blocked, at the parser's real limit: `@every 2562047h` saves and `@every 2562048h` blocks. On K3s this round: 0 snapshots, no error.	No snapshots, no errorK3s	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
`@every` with zero or under a second `@every 0s@every 500ms` About 70 snapshots in 4 minutes on K3s. Valid, but rarely what anyone means. The PR now behaves like master here.	A snapshot every few secondsK3s	➖ Saves	➖ Blocks	➖ Saveshint: Invalid
`@every` with a negative duration `@every -1h` Same burst of snapshots. The PR has a test row that blocks it. That differs from the parser and is the better result.	A snapshot every few secondsK3s	➖ Saves	➖ Blocks	➖ Blockshint: Invalid
`?` in both day fields or in minute or hour `0 0 ? * ?? ? ? ? ?0 ? * * 0 0 ? ? ` Now saves. These fired on K3s in round one.	Snapshots runK3s	✅ Saves	❌ Blocks	✅ Saveshint: Invalid
Stray commas `1,,3 * * * 1, * * *` Now saves. These fired on K3s in round one.	Snapshots runK3s	✅ Saves	❌ Blocks	✅ Saveshint: Invalid
Other odd forms that happen to parse `+1 * * * *-5 * * ?/2 * * @every .5h@every 1.hTZ=UTC @daily` Now saves, so I ran them this round: `-5`, `?/2`, a leading `+` and a `.5` duration took snapshots on K3s and RKE2. A `1.` duration and a trailing space after a timezone did on K3s.	Snapshots runK3s and RKE2 for some, parser for the rest	✅ Saves	❌ Blocks	✅ Saveshint: Invalid
Crash: master saves these, and the K3s or RKE2 server stops at startup
Timezone with nothing after it, or a tab instead of the space `TZ=UTCCRON_TZ=UTCTZ=UTC<tab>@daily` The parser panics rather than returning an error, and the server exits. In the generated comparison 10,449 schedules panic the parser, and the PR saves none of them.	Server stops at startupK3s and RKE2	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
Broken: master saves these, and the cluster takes no snapshots
Six or four fields `* * * * * ** * * 0 0 * * 2026` The issue's case.	No snapshots, no errorK3s for some, parser for the rest	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
Out of range `60 * * * 0 24 * 0 0 0 0 0 32 0 0 0 0 0 13 0 0 * 8/0 * * -1 * * *`	No snapshots, no errorK3s for some, parser for the rest	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
Backward ranges `5-1 * * * 0 22-2 * 0 0 * FRI-MON`	No snapshots, no errorParser only	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
Other cron dialects `0 0 L * 0 0 * 5L0 0 * * 1#10 0 15W * H * * *@reboot`	No snapshots, no errorK3s for some, parser for the rest	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
Not numbers `1.5 * * * 0x10 * * *`	No snapshots, no errorParser only	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
Malformed `@every` `@every@every 90@every 1d@every 6h`	No snapshots, no errorK3s for some, parser for the rest	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
Timezone prefix on a broken schedule `CRON_TZ=UTC * * * * * *`	No snapshots, no errorParser only	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
Shortcut in the wrong case, or made up `@DAILY@Daily@foo`	No snapshots, no errorParser only	❌ Saves	✅ Blocks	✅ Blockshint: Invalid
Not a schedule, or only spaces `not-a-crondaily(spaces)(spaces)`	No snapshots, no errorParser only	❌ Saves	✅ Blocks	✅ Blockshint: Invalid

How this was measured: I typed all 105 schedules into the real Create form on master and on this build, and into Edit and imported on this build. The round-one column is from the same run at 405c0e2. Each row's schedules gave identical results on every build. Under "On the cluster", small text says what the result rests on. These came from test servers in containers using Rancher's config file, not clusters provisioned through Rancher.

Findings

It's now a hand-written parser. Is that what you want?

shell/utils/validators/etcd-snapshot-schedule.ts (new, 114 lines) and its test (119 lines)

Your call

What happened

Your two comments needed about 15 lines around cron-validator, which is what my suggested fix did. Cody's reply says every row of the table is now a test, including the odd forms. cron-validator can't express those, so my read is that's why the parser is hand-written. Cody didn't say so. If that's right, the table drove this. You wrote that some edge cases seemed OK, but the table marked forms like 1,,3 * * * * as ❌, and a ❌ reads as something to fix. I should have marked those rows as not worth fixing.

It gets 4 more rows right than my fix: the absurdly long @every, ? in odd places, stray commas and the other odd forms. Nobody writes any of those. In all it answers 13 of the 105 schedules differently from my fix.

Why I'd still accept it

The grammar isn't moving. K3s main still pins robfig/cron v3.0.1, from January 2020, and RKE2 gets it through K3s. The repo's last commit is from January 2021. So this copy is unlikely to drift.
It's correct. See the parser comparison below.
It reads plainly, field bounds, then ranges, then durations, and the tests caught 37 of 41 deliberate breakages.
Asking for less now costs a round to remove code that works.

Why you might not

Issue 13202 (open, v2.18.0) wants cron validation merged into one, and this adds another with a different grammar from the hint next to it. If you'd rather have less code, the 15-line version is still deployed as dashboard-pr19766fix. Say so and I'll draft the comment.

Three lines that can't change any answer

etcd-snapshot-schedule.ts:65-67

Optional nit

When a range has a step but no high value, this sets end to the maximum. In the Go parser that decides which bits get set. Here end only feeds the range check on the next line, and N/step passes or fails that check the same way whether end is N or the maximum.

I deleted the branch and reran the whole comparison: 0 of 1,731,642 answers changed. It's also the one mutation here that no test could ever catch. It's harmless, and it does keep the code closer to the Go original, so this is only worth mentioning alongside an approve.

Noted, no action needed

FYI

The grey hint and the block disagree on 26 of the 105 schedules. 22 save while the hint says "Invalid cron schedule". 4 block while the hint reads fine, all day of week 7. The hint is unchanged from master, so only those 4 are new, and the tooltip explains them. Cody disclosed this in the description. Cody's reply says it waits for rancher-components, but LabeledInput already has a showCronHint prop and a cron-hint-change event, which the workload form uses, so a form could replace the hint today. I wouldn't ask for that on forms this rare. I found no open issue for the "planned cron input". The closest is issue 13202.
New file under shell/utils/. Section 9 of our review checklist says not to add one. I wouldn't raise it: shell/utils/validators/ already holds per-resource validators like machine-pool.ts, no model owns a cron grammar, and moving it into its only caller would double that composable.
Stored broken schedules block Save until replaced. The description says so, and it's correct now that only schedules that really fail are affected. With 0 0 * * 7 or TZ=UTC stored, Save is disabled and the etcd tab is flagged. Edit as YAML stays available.
Three small things the tests don't pin: SAT as a day name, and the two spellings of µs. All work. They're the other 3 of the 4 breakages the tests missed. Not worth asking for.

The new validator against the real parser

The PR now claims to match what K3s and RKE2 parse, so this tests that claim directly. Of 1,731,642 schedules, 1,726,175 get the same answer. These are all the differences:

Where they differ	Count	Example	What it means
The PR saves, the parser rejects	3,562
A timezone name that doesn't exist	932	`TZ=Nope @daily`	On purpose, and in the code comment. This is the one a person could type.
A step too big for a 64-bit integer	1,313	`/99999999999999999999 * * *`	Nobody writes it. K3s: 0 snapshots.
A byte order mark used as a space	1,279	U+FEFF before `* * * * *`	JavaScript treats it as whitespace and Go doesn't. K3s: 0 snapshots. Only a paste from an odd file could cause it.
Within rounding of the longest duration	38	`@every 2562047h47m16.854775808s`	One nanosecond past the limit, about 292 years.
The PR blocks, the parser accepts	1,905
U+0085 used as a space	1,303	U+0085 between fields	Whitespace in Go only. Nobody types it.
An empty timezone	248	`TZ= * * * * *`	Go reads it as UTC, and K3s took 4 snapshots. Blocking it is harmless.
A field of only commas	258	`, * * * *`	Mostly never fires (K3s: 0 snapshots), so blocking is right. 29 fire through the other day field.
`@every` with a sign	76	`@every +1h`, `@every -1h`	Nobody writes the plus. The minus is the debatable row in the list.
`@every 0` with no unit	19	`@every 0`	It would mean every second, so blocking is fine.
A letter that lowercases differently in Go	1	`0 0 * * FRİ`	A Turkish dotted İ. Nobody types it on purpose.

On real servers, this round

Schedule	PR form	K3s v1.37.1 snapshots	RKE2 v1.37.1 snapshots
`-5 * * *`	Saves	4	4
`?/2 * * * *`	Saves	2	2
`+0/1 * * * *`	Saves	4	4
`@every .5m`	Saves	7	8
`@every 1.m`	Saves	3
`TZ=UTC @every 1m`	Saves	3
`* * * * *` (control)	Saves	4	4
`, * * * *`	Blocks	0
`TZ= * * * * *`	Blocks	4
`/99999999999999999999 * * *`	Saves	0
U+FEFF, then `* * * * *`	Saves	0
`@every 999999999h`	Blocks	0

17 more throwaway servers: 12 on K3s v1.37.1 and 5 on RKE2 v1.37.1, mf's default versions, which makes 84 with round one. Each had the schedule in config.yaml.d/50-rancher.yaml, and all stayed running with no cron error in their logs. K3s ran for about 4 minutes. RKE2 ran until the control had 3 snapshots, plus a minute. RKE2 had no CNI, so its node stayed NotReady, which doesn't affect etcd snapshots. Red means the form's answer doesn't match what the server did. Those are the same rare forms as in the comparison below.

Live results

Case	Result on the new build
105 schedules typed into Create Custom, Edit and imported	All three give the same answers: 61 save, 44 block. The icon and tab flag appear on exactly the blocked ones. Each differs from the real parser only on `TZ=Nope` and `@every -1h`.
The same 105 on master	All 105 save, with no icon or flag. Same hint text as the PR on all 105.
Amazon EC2: default schedule, then six typed	`* * * * * `, `0 0 * 7` and `TZ=UTC` disable Create and flag the tab. `@every 6h`, `CRON_TZ=UTC 0 0 * * ` and `0 /5 * * *` enable it.
What Create sends, captured and stopped	8 schedules, byte for byte, including the trailing space in `TZ=UTC @daily` .
What Edit sends for stored `@every 6h`	Save enabled on load (round one: disabled). The PUT carries it unchanged.
What imported sends, the Save round one never reached	`PUT /v3/clusters/c-p2bmg` carries `@every 6h`, `CRON_TZ=UTC 0 0 * * ` and `0 /5 * * ` unchanged. `0 0 * 7` and `TZ=UTC` keep Save disabled and send nothing.
Real keystrokes, not a paste	5 schedules typed key by key end in the right state, with no uncaught errors from the partial values.
Stored values on load *	`0 0 * * 7` and `TZ=UTC` disable Save and flag the tab. `TZ=UTC 0 0 * * *` and `@every 0s` leave it enabled.
Automatic Backups off, then on again	`TZ=UTC`, `0 0 * * 7` and `* * * * * *`: disabled, enabled, disabled.
S3 and backups flows on Create, 38 steps from round one	Only 4 steps changed, all intended: `0 0 * * 7` and `0 0 * * 1-7` now block, `@every 6h` and `CRON_TZ=UTC 0 0 * * *` now save. The S3 re-select bug stays gone.
Imported: Day 2 Ops off and on, collapsed section, S3, remount, stored values *	All correct. Compared by hand with round one's table, only the same intended changes differ.
View mode with a stored invalid schedule	No icon and no tab flag. The grey hint still shows, same as master.
Edit as YAML with a stored invalid schedule	Form Save disabled, YAML button and YAML Save enabled.
Simplified Chinese	The UI switches. The tooltip falls back to the English text, with no missing-key text. Create is disabled. Expected for a new key.
Clone by URL	Lands on the provider picker on both master and the PR, so it never reaches this form. Not a PR issue.

* Stored values come from a rewritten GET response, except * * * * * * and @every 6h on the Edit fixtures, which are really stored.

What else was checked

Check	Result
Parser comparison	1,731,642 schedules against `robfig/cron` v3.0.1: round one's set, every token sequence up to length 4 in each field, and whole specs built from unusual separators. 1,726,175 agree. 10,449 panic the parser, and the PR saves none of them. No input made the validator throw. The PR saves 20 schedules that parse but never fire, all dates like Feb 31.
Rancher API	A server-side dry run on mf accepted all 22 schedules I tried, including `TZ=UTC` and `0 0 * * 7`, and returned each unchanged. Nothing was created. So Rancher's admission doesn't check this, and the form is the only check before the cluster.
Unit tests	Run locally on the head: validators, composable, etcd forms, `rke2.test.ts` and `pkg/imported`: 13 suites, 466 passed. 2 skipped, from a `describe.skip` that is also on master.
Do the tests catch breakage?	41 deliberate breakages of the validator and composable: 37 fail a test. The other 4 are card 2's dead branch and the three details in card 3.
Other users of the old code	`cron-schedule.js` and its test are back to matching master, so the PR no longer changes them. `cron-validator` was already a dependency and four other files still use it, so nothing is orphaned.
CI	All checks pass. The first attempt failed only `kubewarden.spec.ts` (the live extension catalog), and it passed on the automatic retry. The branch is 5 commits behind master and none of them touch these files.
Code	TypeScript, no `any`, no `v-html`, no issue references in comments. The one new string is in `en-us.yaml`.
Safety	Every write in the browser runs was intercepted. Cluster writes were aborted, and no real resource changed. The dry run stored nothing. All 17 server containers are removed and the Docker VM is back off.
Not tested	A cluster actually provisioned through Rancher. Saves were captured in the browser, so none reached Rancher for real. Servers on a full host OS, since these all ran in containers. `@weekly`, `@monthly`, `@yearly` and `@annually` firing on a server (parser only). Non-admin roles, as the composable has no permission logic.

Suggested review (not posted)

In your voice, in the usual shape: short body, no code blocks, no preview link. Approve. Resolve both threads when you submit, since the fixes are in. Drop the inline if you'd rather not nit. I can add this as a pending review if you want it.

Body

Retested on e11c386c2e on the Create, Edit, Amazon EC2 and imported forms. Day of week 7 blocks, @every and CRON_TZ= save, and TZ=UTC on its own stays blocked. I also ran the new validator against robfig/cron v3.0.1 on about 1.7 million generated schedules. Nothing that crashes the parser gets through, and apart from the timezone name you called out, the differences are things nobody types, like a 20-digit step or a byte order mark. The odd forms took snapshots on K3s and RKE2 test servers too. Looks good.

Optional inline on shell/utils/validators/etcd-snapshot-schedule.ts:65

Optional: I think this branch can go. In robfig end decides which bits get set, but here it only feeds the range check below, and N/step gets the same answer with end at N or at the maximum. Removing it changed none of the answers in my comparison.

Cleanup when the PR merges

Left behind	Remove with
Preview `dashboard-pr19766` on created-dashboard-2	`bash preview/cleanup-branch.sh dashboard pr19766`
Preview `dashboard-pr19766fix` (my round-one fix, not pushed). Only needed if you take the other side of card 1	`bash preview/cleanup-branch.sh dashboard pr19766fix`
Fixture clusters `zz19766-bad-cron` and `zz19766-every` on mf (no nodes)	`kubectl -n fleet-default delete clusters.provisioning.cattle.io zz19766-bad-cron zz19766-every`