Skip to content

Commit

Permalink
fix: Do not allow cron workflow names with more than 52 chars (#5407)
Browse files Browse the repository at this point in the history
Signed-off-by: Simon Behar <simbeh7@gmail.com>
  • Loading branch information
simster7 committed Mar 16, 2021
1 parent 4468c26 commit ae34e4d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
11 changes: 9 additions & 2 deletions workflow/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ const (
anyWorkflowOutputParameterMagicValue = "workflow.outputs.parameters.*"
anyWorkflowOutputArtifactMagicValue = "workflow.outputs.artifacts.*"
maxCharsInObjectName = 63
// CronWorkflows have fewer max chars allowed in their name because when workflows are created from them, they
// are appended with the unix timestamp (`-1615836720`). This lower character allowance allows for that timestamp
// to still fit within the 63 character maximum.
maxCharsInCronWorkflowName = 52
)

var placeholderGenerator = common.NewPlaceholderGenerator()
Expand Down Expand Up @@ -270,8 +274,11 @@ func ValidateClusterWorkflowTemplate(wftmplGetter templateresolution.WorkflowTem

// ValidateCronWorkflow validates a CronWorkflow
func ValidateCronWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, cwftmplGetter templateresolution.ClusterWorkflowTemplateGetter, cronWf *wfv1.CronWorkflow) error {
if len(cronWf.Name) > maxCharsInObjectName {
return fmt.Errorf("cron workflow name %q must not be more than 63 characters long (currently %d)", cronWf.Name, len(cronWf.Name))
// CronWorkflows have fewer max chars allowed in their name because when workflows are created from them, they
// are appended with the unix timestamp (`-1615836720`). This lower character allowance allows for that timestamp
// to still fit within the 63 character maximum.
if len(cronWf.Name) > maxCharsInCronWorkflowName {
return fmt.Errorf("cron workflow name %q must not be more than 52 characters long (currently %d)", cronWf.Name, len(cronWf.Name))
}

if _, err := cron.ParseStandard(cronWf.Spec.Schedule); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions workflow/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2697,7 +2697,7 @@ func TestMaxLengthName(t *testing.T) {
_, err = ValidateClusterWorkflowTemplate(wftmplGetter, cwftmplGetter, cwftmpl)
assert.EqualError(t, err, "cluster workflow template name \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\" must not be more than 63 characters long (currently 70)")

cwf := &wfv1.CronWorkflow{ObjectMeta: metav1.ObjectMeta{Name: strings.Repeat("a", 70)}}
cwf := &wfv1.CronWorkflow{ObjectMeta: metav1.ObjectMeta{Name: strings.Repeat("a", 60)}}
err = ValidateCronWorkflow(wftmplGetter, cwftmplGetter, cwf)
assert.EqualError(t, err, "cron workflow name \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\" must not be more than 63 characters long (currently 70)")
assert.EqualError(t, err, "cron workflow name \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\" must not be more than 52 characters long (currently 60)")
}

0 comments on commit ae34e4d

Please sign in to comment.