fix(tasks): fixes duration validation for every and offset, so people…

… will get feedback if they are using durations that tasks doesn't currently support

task.go

@@ -254,6 +254,14 @@ func (t TaskUpdate) Validate() error {
 	switch {
 	case !t.Options.Every.IsZero() && t.Options.Cron != "":
 		return errors.New("cannot specify both every and cron")
+	case !t.Options.Every.IsZero():
+		if _, err := time.ParseDuration(t.Options.Every.String()); err != nil {
+			return fmt.Errorf("every: %s is invalid", err)
+		}
+	case t.Options.Offset != nil && !t.Options.Offset.IsZero():
+		if _, err := time.ParseDuration(t.Options.Offset.String()); err != nil {
+			return fmt.Errorf("offset: %s, %s is invalid", t.Options.Offset.String(), err)
+		}
 	case t.Flux == nil && t.Status == nil && t.Options.IsZero():
 		return errors.New("cannot update task without content")
 	case t.Status != nil && *t.Status != TaskStatusActive && *t.Status != TaskStatusInactive:

task/options/options.go

@@ -54,8 +54,15 @@ func (a Duration) String() string {
 func (a *Duration) Parse(s string) error {
 	q, err := parseSignedDuration(s)
 	if err != nil {
-		return err
+		return ErrTaskInvalidDuration(err)
+	}
+	// TODO(docmerlin): the following needs to be removed once we can support duration units longer than an hour.
+	// This is here to check to make sure that the duration is compatible with golang durations as well, as the current task
+	// cron doesn't support certain duration units that flux supports. For historical reasons empty-string needs to parse without error
+	if _, err := time.ParseDuration(s); strings.TrimSpace(s) != "" && err != nil {
+		return ErrTaskInvalidDuration(err)
 	}
+
 	a.Node = *q
 	return nil
 }
@@ -135,7 +142,7 @@ func (o *Options) IsZero() bool {
 	return o.Name == "" &&
 		o.Cron == "" &&
 		o.Every.IsZero() &&
-		o.Offset == nil &&
+		(o.Offset == nil || o.Offset.IsZero()) &&
 		o.Concurrency == nil &&
 		o.Retry == nil
 }
@@ -277,9 +284,14 @@ func FromScript(script string) (Options, error) {
 		if err != nil {
 			return opt, err
 		}
+		if _, err := time.ParseDuration(dur.Location().Source); err != nil { // TODO(docmerlin): remove this once tasks fully supports all flux duration units.
+			return opt, ErrParseTaskOptionField("every")
+		}
+
 		if !ok || durNode == nil {
 			return opt, ErrParseTaskOptionField("every")
 		}
+
 		durNode.BaseNode = ast.BaseNode{}
 		opt.Every.Node = *durNode
 	}
@@ -296,6 +308,9 @@ func FromScript(script string) (Options, error) {
 		if err != nil {
 			return opt, err
 		}
+		if _, err := time.ParseDuration(dur.Location().Source); err != nil { // TODO(docmerlin): remove this once tasks fully supports all flux duration units.
+			return opt, ErrParseTaskOptionField("every")
+		}
 		if !ok || durNode == nil {
 			return opt, ErrParseTaskOptionField("offset")
 		}

task/options/options_errors.go

@@ -12,6 +12,11 @@ func ErrMissingRequiredTaskOption(opt string) error {
 	return fmt.Errorf("missing required option: %s", opt)
 }
 
+// ErrTaskInvalidDuration is returned when an "every" or "offset" option is invalid in a task.
+func ErrTaskInvalidDuration(err error) error {
+	return fmt.Errorf("invalid duration in task %s", err)
+}
+
 var (
 	ErrDuplicateIntervalField = fmt.Errorf("cannot use both cron and every in task options")
 )

task/options/options_test.go

@@ -78,6 +78,27 @@ func TestFromScript(t *testing.T) {
 		{script: "option task = {\n  name: \"name8\",\n  retry: 0,\n  every: 1m0s,\n\n}\n\nfrom(bucket: \"test\")\n    |> range(start:-1h)", shouldErr: true},
 		{script: scriptGenerator(options.Options{Name: "name9"}, ""), shouldErr: true},
 		{script: scriptGenerator(options.Options{}, ""), shouldErr: true},
+		{script: `option task = {
+			name: "test",
+			every: 1d,
+			offset: 1m
+		}
+			from(bucket: "metrics")
+			|> range(start: now(), stop: 8w)
+
+		`, shouldErr: true}, // TODO(docmerlin): remove this once tasks fully supports all flux duration units.
+		{script: `option task = {
+			name: "test",
+			every: 1m,
+			offset: 1d
+		}
+			from(bucket: "metrics")
+			|> range(start: now(), stop: 8w)
+
+		`, shouldErr: true}, // TODO(docmerlin): remove this once tasks fully supports all flux duration units.
+		{script: "option task = {name:\"test_task_smoke_name\", every:30s} from(bucket:\"test_tasks_smoke_bucket_source\") |> range(start: -1h) |> map(fn: (r) => ({r with _time: r._time, _value:r._value, t : \"quality_rocks\"}))|> to(bucket:\"test_tasks_smoke_bucket_dest\", orgID:\"3e73e749495d37d5\")",
+			exp: options.Options{Name: "test_task_smoke_name", Every: *(options.MustParseDuration("30s")), Retry: pointer.Int64(1), Concurrency: pointer.Int64(1)}, shouldErr: false}, // TODO(docmerlin): remove this once tasks fully supports all flux duration units.
+
 	} {
 		o, err := options.FromScript(c.script)
 		if c.shouldErr && err == nil {