From 8f70d2243e07c04254222b1cabf8088245ca55e2 Mon Sep 17 00:00:00 2001 From: Simon Behar Date: Thu, 30 Jul 2020 08:28:06 -0700 Subject: [PATCH] fix: Don't panic on invalid template creation (#3643) --- workflow/common/util.go | 10 ++++++++-- workflow/controller/dag.go | 10 ++++++++-- workflow/controller/operator.go | 17 ++++++++++++++--- workflow/controller/steps.go | 10 ++++++++-- workflow/controller/workflowpod.go | 10 ++++++++-- workflow/validate/validate.go | 5 ++++- 6 files changed, 50 insertions(+), 12 deletions(-) diff --git a/workflow/common/util.go b/workflow/common/util.go index 44089adb096d..c719ff8b222f 100644 --- a/workflow/common/util.go +++ b/workflow/common/util.go @@ -307,7 +307,10 @@ func SubstituteParams(tmpl *wfv1.Template, globalParams, localParams Parameters) } // First replace globals & locals, then replace inputs because globals could be referenced in the inputs replaceMap := globalParams.Merge(localParams) - fstTmpl := fasttemplate.New(string(tmplBytes), "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(string(tmplBytes), "{{", "}}") + if err != nil { + return nil, fmt.Errorf("unable to parse argo varaible: %w", err) + } globalReplacedTmplStr, err := Replace(fstTmpl, replaceMap, true) if err != nil { return nil, err @@ -347,7 +350,10 @@ func SubstituteParams(tmpl *wfv1.Template, globalParams, localParams Parameters) } } - fstTmpl = fasttemplate.New(globalReplacedTmplStr, "{{", "}}") + fstTmpl, err = fasttemplate.NewTemplate(globalReplacedTmplStr, "{{", "}}") + if err != nil { + return nil, fmt.Errorf("unable to parse argo varaible: %w", err) + } s, err := Replace(fstTmpl, replaceMap, true) if err != nil { return nil, err diff --git a/workflow/controller/dag.go b/workflow/controller/dag.go index baced0118095..447620d6982f 100644 --- a/workflow/controller/dag.go +++ b/workflow/controller/dag.go @@ -502,7 +502,10 @@ func (woc *wfOperationCtx) resolveDependencyReferences(dagCtx *dagContext, task if err != nil { return nil, errors.InternalWrapError(err) } - fstTmpl := fasttemplate.New(string(taskBytes), "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(string(taskBytes), "{{", "}}") + if err != nil { + return nil, fmt.Errorf("unable to parse argo varaible: %w", err) + } newTaskStr, err := common.Replace(fstTmpl, woc.globalParams.Merge(scope.getParameters()), true) if err != nil { @@ -595,7 +598,10 @@ func expandTask(task wfv1.DAGTask) ([]wfv1.DAGTask, error) { return []wfv1.DAGTask{task}, nil } - fstTmpl := fasttemplate.New(string(taskBytes), "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(string(taskBytes), "{{", "}}") + if err != nil { + return nil, fmt.Errorf("unable to parse argo varaible: %w", err) + } expandedTasks := make([]wfv1.DAGTask, 0) for i, item := range items { var newTask wfv1.DAGTask diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index 6b49e8f57f0a..b84ed82803b2 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -2590,7 +2590,10 @@ func (woc *wfOperationCtx) substituteParamsInVolumes(params map[string]string) e if err != nil { return errors.InternalWrapError(err) } - fstTmpl := fasttemplate.New(string(volumesBytes), "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(string(volumesBytes), "{{", "}}") + if err != nil { + return fmt.Errorf("unable to parse argo varaible: %w", err) + } newVolumesStr, err := common.Replace(fstTmpl, params, true) if err != nil { return err @@ -2659,7 +2662,11 @@ func (woc *wfOperationCtx) computeMetrics(metricList []*wfv1.Prometheus, localSc woc.reportMetricEmissionError(fmt.Sprintf("unable to substitute parameters for metric '%s' (marshal): %s", metricTmpl.Name, err)) continue } - fstTmpl := fasttemplate.New(string(metricTmplBytes), "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(string(metricTmplBytes), "{{", "}}") + if err != nil { + woc.reportMetricEmissionError(fmt.Sprintf("unable to parse argo varaible for metric '%s': %s", metricTmpl.Name, err)) + continue + } replacedValue, err := common.Replace(fstTmpl, localScope, false) if err != nil { woc.reportMetricEmissionError(fmt.Sprintf("unable to substitute parameters for metric '%s': %s", metricTmpl.Name, err)) @@ -2715,7 +2722,11 @@ func (woc *wfOperationCtx) computeMetrics(metricList []*wfv1.Prometheus, localSc metricSpec := metricTmpl.DeepCopy() // Finally substitute value parameters - fstTmpl = fasttemplate.New(metricSpec.GetValueString(), "{{", "}}") + fstTmpl, err = fasttemplate.NewTemplate(metricSpec.GetValueString(), "{{", "}}") + if err != nil { + woc.reportMetricEmissionError(fmt.Sprintf("unable to parse argo varaible for metric '%s': %s", metricTmpl.Name, err)) + continue + } replacedValue, err := common.Replace(fstTmpl, localScope, false) if err != nil { woc.reportMetricEmissionError(fmt.Sprintf("unable to substitute parameters for metric '%s': %s", metricSpec.Name, err)) diff --git a/workflow/controller/steps.go b/workflow/controller/steps.go index 70bbd2b28ca7..c2d24ef22e1e 100644 --- a/workflow/controller/steps.go +++ b/workflow/controller/steps.go @@ -348,7 +348,10 @@ func (woc *wfOperationCtx) resolveReferences(stepGroup []wfv1.WorkflowStep, scop if err != nil { return nil, errors.InternalWrapError(err) } - fstTmpl := fasttemplate.New(string(stepBytes), "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(string(stepBytes), "{{", "}}") + if err != nil { + return nil, fmt.Errorf("unable to parse argo varaible: %w", err) + } newStepStr, err := common.Replace(fstTmpl, woc.globalParams.Merge(scope.getParameters()), true) if err != nil { @@ -431,7 +434,10 @@ func (woc *wfOperationCtx) expandStep(step wfv1.WorkflowStep) ([]wfv1.WorkflowSt if err != nil { return nil, errors.InternalWrapError(err) } - fstTmpl := fasttemplate.New(string(stepBytes), "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(string(stepBytes), "{{", "}}") + if err != nil { + return nil, fmt.Errorf("unable to parse argo varaible: %w", err) + } expandedStep := make([]wfv1.WorkflowStep, 0) var items []wfv1.Item if len(step.WithItems) > 0 { diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go index 8e456f2f7ceb..bbaaf42e4bf4 100644 --- a/workflow/controller/workflowpod.go +++ b/workflow/controller/workflowpod.go @@ -357,7 +357,10 @@ func substitutePodParams(pod *apiv1.Pod, globalParams common.Parameters, tmpl *w if err != nil { return nil, err } - fstTmpl := fasttemplate.New(string(specBytes), "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(string(specBytes), "{{", "}}") + if err != nil { + return nil, fmt.Errorf("unable to parse argo varaible: %w", err) + } newSpecBytes, err := common.Replace(fstTmpl, podParams, true) if err != nil { return nil, err @@ -1111,7 +1114,10 @@ func verifyResolvedVariables(obj interface{}) error { return err } var unresolvedErr error - fstTmpl := fasttemplate.New(string(str), "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(string(str), "{{", "}}") + if err != nil { + return fmt.Errorf("unable to parse argo varaible: %w", err) + } fstTmpl.ExecuteFuncString(func(w io.Writer, tag string) (int, error) { unresolvedErr = errors.Errorf(errors.CodeBadRequest, "failed to resolve {{%s}}", tag) return 0, nil diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go index 800df3cfed8d..35c94369ce90 100644 --- a/workflow/validate/validate.go +++ b/workflow/validate/validate.go @@ -589,7 +589,10 @@ func resolveAllVariables(scope map[string]interface{}, tmplStr string) error { _, allowAllItemRefs := scope[anyItemMagicValue] // 'item.*' is a magic placeholder value set by addItemsToScope _, allowAllWorkflowOutputParameterRefs := scope[anyWorkflowOutputParameterMagicValue] _, allowAllWorkflowOutputArtifactRefs := scope[anyWorkflowOutputArtifactMagicValue] - fstTmpl := fasttemplate.New(tmplStr, "{{", "}}") + fstTmpl, err := fasttemplate.NewTemplate(tmplStr, "{{", "}}") + if err != nil { + return fmt.Errorf("unable to parse argo varaible: %w", err) + } fstTmpl.ExecuteFuncString(func(w io.Writer, tag string) (int, error) {