fix(cli): Validate cron on update. Fixes argoproj#5691 (argoproj#5692)

highb · Apr 19, 2021 · b267e3c · b267e3c
1 parent 9a872de
commit b267e3c
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 22 deletions.
diff --git a/server/cronworkflow/cron_workflow_server.go b/server/cronworkflow/cron_workflow_server.go
@@ -73,10 +73,16 @@ func (c *cronWorkflowServiceServer) GetCronWorkflow(ctx context.Context, req *cr
 }
 
 func (c *cronWorkflowServiceServer) UpdateCronWorkflow(ctx context.Context, req *cronworkflowpkg.UpdateCronWorkflowRequest) (*v1alpha1.CronWorkflow, error) {
+ wfClient := auth.GetWfClient(ctx)
  _, err := c.getCronWorkflowAndValidate(ctx, req.Namespace, req.CronWorkflow.Name, metav1.GetOptions{})
  if err != nil {
  return nil, err
  }
+ wftmplGetter := templateresolution.WrapWorkflowTemplateInterface(wfClient.ArgoprojV1alpha1().WorkflowTemplates(req.Namespace))
+ cwftmplGetter := templateresolution.WrapClusterWorkflowTemplateInterface(wfClient.ArgoprojV1alpha1().ClusterWorkflowTemplates())
+ if err := validate.ValidateCronWorkflow(wftmplGetter, cwftmplGetter, req.CronWorkflow); err != nil {
+ return nil, err
+ }
  return auth.GetWfClient(ctx).ArgoprojV1alpha1().CronWorkflows(req.Namespace).Update(ctx, req.CronWorkflow, metav1.UpdateOptions{})
 }
 

diff --git a/server/cronworkflow/cron_workflow_server_test.go b/server/cronworkflow/cron_workflow_server_test.go
@@ -96,6 +96,12 @@ metadata:
  })
  })
  t.Run("UpdateCronWorkflow", func(t *testing.T) {
+ t.Run("Invalid", func(t *testing.T) {
+ x := cronWf.DeepCopy()
+ x.Spec.Schedule = "invalid"
+ _, err := server.UpdateCronWorkflow(ctx, &cronworkflowpkg.UpdateCronWorkflowRequest{Namespace: "my-ns", CronWorkflow: x})
+ assert.Error(t, err)
+ })
  t.Run("Labelled", func(t *testing.T) {
  cronWf, err := server.UpdateCronWorkflow(ctx, &cronworkflowpkg.UpdateCronWorkflowRequest{Namespace: "my-ns", CronWorkflow: &cronWf})
  if assert.NoError(t, err) {

diff --git a/ui/src/app/cron-workflows/components/cron-workflow-status-viewer.tsx b/ui/src/app/cron-workflows/components/cron-workflow-status-viewer.tsx
@@ -6,7 +6,6 @@ import {ConditionsPanel} from '../../shared/conditions-panel';
 import {WorkflowLink} from '../../workflows/components/workflow-link';
 import {PrettySchedule} from './pretty-schedule';
 
-const parser = require('cron-parser');
 export const CronWorkflowStatusViewer = ({spec, status}: {spec: CronWorkflowSpec; status: CronWorkflowStatus}) => {
  if (status === null) {
  return null;
@@ -25,14 +24,6 @@ export const CronWorkflowStatusViewer = ({spec, status}: {spec: CronWorkflowSpec
  )
  },
  {title: 'Last Scheduled Time', value: <Timestamp date={status.lastScheduledTime} />},
- {
- title: 'Next Scheduled Time',
- value: (
- <>
- <Timestamp date={getNextScheduledTime(spec.schedule, spec.timezone)} /> (assumes workflow-controller is in UTC)
- </>
- )
- },
  {title: 'Conditions', value: <ConditionsPanel conditions={status.conditions} />}
  ].map(attr => (
  <div className='row white-box__details-row' key={attr.title}>
@@ -48,16 +39,3 @@ export const CronWorkflowStatusViewer = ({spec, status}: {spec: CronWorkflowSpec
 function getCronWorkflowActiveWorkflowList(active: kubernetes.ObjectReference[]) {
  return active.reverse().map(activeWf => <WorkflowLink key={activeWf.uid} namespace={activeWf.namespace} name={activeWf.name} />);
 }
-
-function getNextScheduledTime(schedule: string, tz: string): string {
- let out = '';
- try {
- out = parser
- .parseExpression(schedule, {utc: !tz, tz})
- .next()
- .toISOString();
- } catch (e) {
- // Do nothing
- }
- return out;
-}
diff --git a/ui/src/app/cron-workflows/components/schedule-validator.tsx b/ui/src/app/cron-workflows/components/schedule-validator.tsx
@@ -5,6 +5,9 @@ const x = require('cronstrue');
 
 export const ScheduleValidator = ({schedule}: {schedule: string}) => {
  try {
+ if (schedule.split(' ').length === 6) {
+ throw new Error('seconds are not supported');
+ }
  return (
  <span>
  <SuccessIcon /> {x.toString(schedule)}