From 2a2ecc916925642fd8cb1efd026588e6828f82e1 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw Date: Thu, 5 Aug 2021 11:24:41 -0400 Subject: [PATCH] fix(controller): JSON-unmarshal marshaled expression template before evaluating (#6285) Signed-off-by: Michael Crenshaw --- docs/fields.md | 24 ++++++ .../expression-reusing-verbose-snippets.yaml | 44 ++++++++++ util/template/expression_template.go | 29 ++++++- util/template/replace.go | 22 ++++- util/template/replace_test.go | 70 +++++++++------- util/template/template_test.go | 83 +++++++++++++++++++ workflow/controller/operator.go | 20 ++++- workflow/controller/scope.go | 19 ++++- 8 files changed, 270 insertions(+), 41 deletions(-) create mode 100644 examples/expression-reusing-verbose-snippets.yaml create mode 100644 util/template/template_test.go diff --git a/docs/fields.md b/docs/fields.md index 0eb8d36ad86b..398b4c2eaaae 100644 --- a/docs/fields.md +++ b/docs/fields.md @@ -124,6 +124,8 @@ Workflow is the definition of a workflow resource - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`expression-tag-template-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-tag-template-workflow.yaml) - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) @@ -527,6 +529,8 @@ WorkflowSpec is the specification of a Workflow. - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`expression-tag-template-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-tag-template-workflow.yaml) - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) @@ -947,6 +951,8 @@ CronWorkflowSpec is the specification of a CronWorkflow - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`expression-tag-template-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-tag-template-workflow.yaml) - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) @@ -1324,6 +1330,8 @@ WorkflowTemplateSpec is a spec of WorkflowTemplate. - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`expression-tag-template-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-tag-template-workflow.yaml) - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) @@ -1660,6 +1668,8 @@ Arguments to a template - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`expression-tag-template-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-tag-template-workflow.yaml) - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) @@ -2387,6 +2397,8 @@ Parameter indicate a passed string parameter to a service template with an optio - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`expression-tag-template-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-tag-template-workflow.yaml) - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) @@ -2901,6 +2913,8 @@ Inputs are the mechanism for passing parameters, artifacts, volumes from one tem - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`expression-tag-template-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-tag-template-workflow.yaml) - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) @@ -3106,6 +3120,8 @@ ScriptTemplate is a template subtype to enable scripting through code steps - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) - [`loops-param-result.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/loops-param-result.yaml) @@ -4077,6 +4093,8 @@ DataSource sources external data into a data template - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) - [`loops-param-result.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/loops-param-result.yaml) @@ -4708,6 +4726,8 @@ ObjectMeta is metadata that all persisted resources must have, which includes al - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`expression-tag-template-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-tag-template-workflow.yaml) - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) @@ -5584,6 +5604,8 @@ EnvVar represents an environment variable present in a Container. - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`secrets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/secrets.yaml) - [`sidecar-dind.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/sidecar-dind.yaml) @@ -5989,6 +6011,8 @@ PersistentVolumeClaimSpec describes the common attributes of storage devices and - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-destructure-json.yaml) +- [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-reusing-verbose-snippets.yaml) + - [`expression-tag-template-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/expression-tag-template-workflow.yaml) - [`fibonacci-seq-conditional-param.yaml`](https://github.com/argoproj/argo-workflows/blob/master/examples/fibonacci-seq-conditional-param.yaml) diff --git a/examples/expression-reusing-verbose-snippets.yaml b/examples/expression-reusing-verbose-snippets.yaml new file mode 100644 index 000000000000..a51815c2a42e --- /dev/null +++ b/examples/expression-reusing-verbose-snippets.yaml @@ -0,0 +1,44 @@ +# Expressions do not support variables. Rather than repeating verbose expressions to reuse output, you can map over the +# expression output and use its value which is aliased as `#`. Then you can place your "variables" in a JSON object to +# be used elsewhere. +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: expression-reusing-verbose-snippets- +spec: + arguments: + parameters: + - name: weather + # The base64 string is this JSON: {"temps": [34, 27, 15, 57, 46]} + value: '{"weekWeather": "eyJ0ZW1wcyI6IFszNCwgMjcsIDE1LCA1NywgNDZdfQo="}' + entrypoint: main + templates: + - name: main + inputs: + parameters: + - name: week-temps + # The line being mapped over is verbose. Rather than repeat it, we use `map` to alias its output as #. + value: >- + {{= + map([ + jsonpath(sprig.b64dec(jsonpath(workflow.parameters.weather, '$.weekWeather')), '$.temps') + ], { + toJson({ + avg: sprig.add(#[0], #[1], #[2], #[3], #[4]) / 5, + min: sprig.min(#[0], #[1], #[2], #[3], #[4]), + max: sprig.max(#[0], #[1], #[2], #[3], #[4]) + }) + })[0] + }} + script: + env: + - name: AVG + value: "{{=jsonpath(inputs.parameters['week-temps'], '$.avg')}}" + - name: MIN + value: "{{=jsonpath(inputs.parameters['week-temps'], '$.min')}}" + - name: MAX + value: "{{=jsonpath(inputs.parameters['week-temps'], '$.max')}}" + image: debian:9.4 + command: [bash] + source: | + echo "The week's average temperature was $AVG with a minimum of $MIN and a maximum of $MAX." diff --git a/util/template/expression_template.go b/util/template/expression_template.go index fa9d8fd079f9..219ef55abe14 100644 --- a/util/template/expression_template.go +++ b/util/template/expression_template.go @@ -1,6 +1,7 @@ package template import ( + "encoding/json" "fmt" "io" "os" @@ -17,12 +18,22 @@ func init() { } func expressionReplace(w io.Writer, expression string, env map[string]interface{}, allowUnresolved bool) (int, error) { - if _, ok := env["retries"]; !ok && hasRetries(expression) && allowUnresolved { + // The template is JSON-marshaled. This JSON-unmarshals the expression to undo any character escapes. + var unmarshalledExpression string + err := json.Unmarshal([]byte(fmt.Sprintf(`"%s"`, expression)), &unmarshalledExpression) + if err != nil && allowUnresolved { + return w.Write([]byte(fmt.Sprintf("{{%s%s}}", kindExpression, expression))) + } + if err != nil { + return 0, fmt.Errorf("failed to unmarshall JSON expression: %w", err) + } + + if _, ok := env["retries"]; !ok && hasRetries(unmarshalledExpression) && allowUnresolved { // this is to make sure expressions like `sprig.int(retries)` don't get resolved to 0 when `retries` don't exist in the env // See https://github.com/argoproj/argo-workflows/issues/5388 return w.Write([]byte(fmt.Sprintf("{{%s%s}}", kindExpression, expression))) } - result, err := expr.Eval(expression, env) + result, err := expr.Eval(unmarshalledExpression, env) if (err != nil || result == nil) && allowUnresolved { // result is also un-resolved, and any error can be unresolved return w.Write([]byte(fmt.Sprintf("{{%s%s}}", kindExpression, expression))) } @@ -32,7 +43,19 @@ func expressionReplace(w io.Writer, expression string, env map[string]interface{ if result == nil { return 0, fmt.Errorf("failed to evaluate expression %q", expression) } - return w.Write([]byte(fmt.Sprintf("%v", result))) + resultMarshaled, err := json.Marshal(fmt.Sprintf("%v", result)) + if (err != nil || resultMarshaled == nil) && allowUnresolved { + return w.Write([]byte(fmt.Sprintf("{{%s%s}}", kindExpression, expression))) + } + if err != nil { + return 0, fmt.Errorf("failed to marshal evaluated expression: %w", err) + } + if resultMarshaled == nil { + return 0, fmt.Errorf("failed to marshal evaluated marshaled expression %q", expression) + } + // Trim leading and trailing quotes. The value is being inserted into something that's already a string. + marshaledLength := len(resultMarshaled) + return w.Write(resultMarshaled[1 : marshaledLength-1]) } func envMap(replaceMap map[string]string) map[string]interface{} { diff --git a/util/template/replace.go b/util/template/replace.go index 79950dcc1c85..7a38a29d13b9 100644 --- a/util/template/replace.go +++ b/util/template/replace.go @@ -1,9 +1,29 @@ package template +import ( + "encoding/json" + "errors" +) + +// Replace takes a json-formatted string and performs variable replacement. func Replace(s string, replaceMap map[string]string, allowUnresolved bool) (string, error) { + if !json.Valid([]byte(s)) { + return "", errors.New("cannot do template replacements with invalid JSON") + } + t, err := NewTemplate(s) if err != nil { return "", err } - return t.Replace(replaceMap, allowUnresolved) + + replacedString, err := t.Replace(replaceMap, allowUnresolved) + if err != nil { + return s, err + } + + if !json.Valid([]byte(replacedString)) { + return s, errors.New("cannot finish template replacement because the result was invalid JSON") + } + + return replacedString, nil } diff --git a/util/template/replace_test.go b/util/template/replace_test.go index 5775424bdd2e..a8c28c72dabb 100644 --- a/util/template/replace_test.go +++ b/util/template/replace_test.go @@ -1,56 +1,62 @@ package template import ( + "encoding/json" "testing" "github.com/stretchr/testify/assert" ) +func toJsonString(v interface{}) string { + jsonString, _ := json.Marshal(v) + return string(jsonString) +} + func Test_Replace(t *testing.T) { - t.Run("InvailedTemplate", func(t *testing.T) { - _, err := Replace("{{", nil, false) + t.Run("InvalidTemplate", func(t *testing.T) { + _, err := Replace(toJsonString("{{"), nil, false) assert.Error(t, err) }) t.Run("Simple", func(t *testing.T) { t.Run("Valid", func(t *testing.T) { - r, err := Replace("{{foo}}", map[string]string{"foo": "bar"}, false) + r, err := Replace(toJsonString("{{foo}}"), map[string]string{"foo": "bar"}, false) assert.NoError(t, err) - assert.Equal(t, "bar", r) + assert.Equal(t, toJsonString("bar"), r) }) t.Run("Unresolved", func(t *testing.T) { t.Run("Allowed", func(t *testing.T) { - _, err := Replace("{{foo}}", nil, true) + _, err := Replace(toJsonString("{{foo}}"), nil, true) assert.NoError(t, err) }) t.Run("Disallowed", func(t *testing.T) { - _, err := Replace("{{foo}}", nil, false) + _, err := Replace(toJsonString("{{foo}}"), nil, false) assert.EqualError(t, err, "failed to resolve {{foo}}") }) }) }) t.Run("Expression", func(t *testing.T) { t.Run("Valid", func(t *testing.T) { - r, err := Replace("{{=foo}}", map[string]string{"foo": "bar"}, false) + r, err := Replace(toJsonString("{{=foo}}"), map[string]string{"foo": "bar"}, false) assert.NoError(t, err) - assert.Equal(t, "bar", r) + assert.Equal(t, toJsonString("bar"), r) }) t.Run("Unresolved", func(t *testing.T) { t.Run("Allowed", func(t *testing.T) { - _, err := Replace("{{=foo}}", nil, true) + _, err := Replace(toJsonString("{{=foo}}"), nil, true) assert.NoError(t, err) }) t.Run("AllowedRetries", func(t *testing.T) { - replaced, err := Replace("{{=sprig.int(retries)}}", nil, true) + replaced, err := Replace(toJsonString("{{=sprig.int(retries)}}"), nil, true) assert.NoError(t, err) - assert.Equal(t, replaced, "{{=sprig.int(retries)}}") + assert.Equal(t, replaced, toJsonString("{{=sprig.int(retries)}}")) }) t.Run("Disallowed", func(t *testing.T) { - _, err := Replace("{{=foo}}", nil, false) + _, err := Replace(toJsonString("{{=foo}}"), nil, false) assert.EqualError(t, err, "failed to evaluate expression \"foo\"") }) }) t.Run("Error", func(t *testing.T) { - _, err := Replace("{{=!}}", nil, false) + _, err := Replace(toJsonString("{{=!}}"), nil, false) if assert.Error(t, err) { assert.Contains(t, err.Error(), "failed to evaluate expression") } @@ -61,53 +67,53 @@ func Test_Replace(t *testing.T) { func TestNestedReplaceString(t *testing.T) { replaceMap := map[string]string{"inputs.parameters.message": "hello world"} - test := `{{- with secret "{{inputs.parameters.message}}" -}} + test := toJsonString(`{{- with secret "{{inputs.parameters.message}}" -}} {{ .Data.data.gitcreds }} - {{- end }}` + {{- end }}`) replacement, err := Replace(test, replaceMap, true) if assert.NoError(t, err) { - assert.Equal(t, "{{- with secret \"hello world\" -}}\n {{ .Data.data.gitcreds }}\n {{- end }}", replacement) + assert.Equal(t, toJsonString("{{- with secret \"hello world\" -}}\n {{ .Data.data.gitcreds }}\n {{- end }}"), replacement) } - test = `{{- with {{ secret "{{inputs.parameters.message}}" -}} + test = toJsonString(`{{- with {{ secret "{{inputs.parameters.message}}" -}} {{ .Data.data.gitcreds }} - {{- end }}` + {{- end }}`) replacement, err = Replace(test, replaceMap, true) if assert.NoError(t, err) { - assert.Equal(t, "{{- with {{ secret \"hello world\" -}}\n {{ .Data.data.gitcreds }}\n {{- end }}", replacement) + assert.Equal(t, toJsonString("{{- with {{ secret \"hello world\" -}}\n {{ .Data.data.gitcreds }}\n {{- end }}"), replacement) } - test = `{{- with {{ secret "{{inputs.parameters.message}}" -}} }} + test = toJsonString(`{{- with {{ secret "{{inputs.parameters.message}}" -}} }} {{ .Data.data.gitcreds }} - {{- end }}` + {{- end }}`) replacement, err = Replace(test, replaceMap, true) if assert.NoError(t, err) { - assert.Equal(t, "{{- with {{ secret \"hello world\" -}} }}\n {{ .Data.data.gitcreds }}\n {{- end }}", replacement) + assert.Equal(t, toJsonString("{{- with {{ secret \"hello world\" -}} }}\n {{ .Data.data.gitcreds }}\n {{- end }}"), replacement) } - test = `{{- with secret "{{inputs.parameters.message}}" -}} }} + test = toJsonString(`{{- with secret "{{inputs.parameters.message}}" -}} }} {{ .Data.data.gitcreds }} - {{- end }}` + {{- end }}`) replacement, err = Replace(test, replaceMap, true) if assert.NoError(t, err) { - assert.Equal(t, "{{- with secret \"hello world\" -}} }}\n {{ .Data.data.gitcreds }}\n {{- end }}", replacement) + assert.Equal(t, toJsonString("{{- with secret \"hello world\" -}} }}\n {{ .Data.data.gitcreds }}\n {{- end }}"), replacement) } - test = `{{- with {{ {{ }} secret "{{inputs.parameters.message}}" -}} }} + test = toJsonString(`{{- with {{ {{ }} secret "{{inputs.parameters.message}}" -}} }} {{ .Data.data.gitcreds }} - {{- end }}` + {{- end }}`) replacement, err = Replace(test, replaceMap, true) if assert.NoError(t, err) { - assert.Equal(t, "{{- with {{ {{ }} secret \"hello world\" -}} }}\n {{ .Data.data.gitcreds }}\n {{- end }}", replacement) + assert.Equal(t, toJsonString("{{- with {{ {{ }} secret \"hello world\" -}} }}\n {{ .Data.data.gitcreds }}\n {{- end }}"), replacement) } - test = `{{- with {{ {{ }} secret "{{does-not-exist}}" -}} }} + test = toJsonString(`{{- with {{ {{ }} secret "{{does-not-exist}}" -}} }} {{ .Data.data.gitcreds }} - {{- end }}` + {{- end }}`) replacement, err = Replace(test, replaceMap, true) if assert.NoError(t, err) { @@ -118,9 +124,9 @@ func TestNestedReplaceString(t *testing.T) { func TestReplaceStringWithWhiteSpace(t *testing.T) { replaceMap := map[string]string{"inputs.parameters.message": "hello world"} - test := `{{ inputs.parameters.message }}` + test := toJsonString(`{{ inputs.parameters.message }}`) replacement, err := Replace(test, replaceMap, true) if assert.NoError(t, err) { - assert.Equal(t, "hello world", replacement) + assert.Equal(t, toJsonString("hello world"), replacement) } } diff --git a/util/template/template_test.go b/util/template/template_test.go new file mode 100644 index 000000000000..f108c90cd39e --- /dev/null +++ b/util/template/template_test.go @@ -0,0 +1,83 @@ +package template + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +type SimpleValue struct { + Value string `json:"value,omitempty"` +} + +func processTemplate(t *testing.T, tmpl SimpleValue) SimpleValue { + tmplBytes, err := json.Marshal(tmpl) + assert.NoError(t, err) + r, err := Replace(string(tmplBytes), map[string]string{}, true) + assert.NoError(t, err) + var newTmpl SimpleValue + err = json.Unmarshal([]byte(r), &newTmpl) + assert.NoError(t, err) + return newTmpl +} + +func Test_Template_Replace(t *testing.T) { + t.Run("ExpressionWithEscapedCharacters", func(t *testing.T) { + t.Run("SingleQuotes", func(t *testing.T) { + tmpl := SimpleValue{Value: "{{='test'}}"} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, "test", newTmpl.Value) + }) + t.Run("DoubleQuotes", func(t *testing.T) { + tmpl := SimpleValue{Value: `{{="test"}}`} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, "test", newTmpl.Value) + }) + t.Run("EscapedBackslashInString", func(t *testing.T) { + tmpl := SimpleValue{Value: `{{='some\\path\\with\\backslashes'}}`} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, `some\path\with\backslashes`, newTmpl.Value) + }) + t.Run("EscapedNewlineInString", func(t *testing.T) { + tmpl := SimpleValue{Value: `{{='some\nstring\nwith\nescaped\nnewlines'}}`} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, "some\nstring\nwith\nescaped\nnewlines", newTmpl.Value) + }) + t.Run("Newline", func(t *testing.T) { + tmpl := SimpleValue{Value: "{{=1 + \n1}}"} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, "2", newTmpl.Value) + }) + t.Run("StringAsJson", func(t *testing.T) { + tmpl := SimpleValue{Value: "{{=toJson('test')}}"} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, `"test"`, newTmpl.Value) + }) + t.Run("ObjectAsJson", func(t *testing.T) { + tmpl := SimpleValue{Value: "{{=toJson({test: 1})}}"} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, `{"test":1}`, newTmpl.Value) + }) + t.Run("ArrayAsJson", func(t *testing.T) { + tmpl := SimpleValue{Value: "{{=toJson([1, '2', {an: 'object'}])}}"} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, `[1,"2",{"an":"object"}]`, newTmpl.Value) + }) + t.Run("SingleQuoteAsString", func(t *testing.T) { + tmpl := SimpleValue{Value: `{{="'"}}`} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, `'`, newTmpl.Value) + }) + t.Run("DoubleQuoteAsString", func(t *testing.T) { + tmpl := SimpleValue{Value: `{{='"'}}`} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, `"`, newTmpl.Value) + }) + t.Run("Boolean", func(t *testing.T) { + tmpl := SimpleValue{Value: `{{=true == false}}`} + newTmpl := processTemplate(t, tmpl) + assert.Equal(t, "false", newTmpl.Value) + }) + }) +} diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index 589b3b51527c..7c1b13c23cf4 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -3154,12 +3154,28 @@ func (woc *wfOperationCtx) computeMetrics(metricList []*wfv1.Prometheus, localSc metricSpec := metricTmpl.DeepCopy() // Finally substitute value parameters - replacedValue, err := template.Replace(metricSpec.GetValueString(), localScope, false) + metricValueString := metricSpec.GetValueString() + + metricValueStringJson, err := json.Marshal(metricValueString) + if err != nil { + woc.reportMetricEmissionError(fmt.Sprintf("unable to marshal metric to JSON for templating '%s': %s", metricSpec.Name, err)) + continue + } + + replacedValueJson, err := template.Replace(string(metricValueStringJson), localScope, false) if err != nil { woc.reportMetricEmissionError(fmt.Sprintf("unable to substitute parameters for metric '%s': %s", metricSpec.Name, err)) continue } - metricSpec.SetValueString(replacedValue) + + var replacedStringJson string + err = json.Unmarshal([]byte(replacedValueJson), &replacedStringJson) + if err != nil { + woc.reportMetricEmissionError(fmt.Sprintf("unable to unmarshal templated metric JSON '%s': %s", metricSpec.Name, err)) + continue + } + + metricSpec.SetValueString(replacedStringJson) metric := woc.controller.metrics.GetCustomMetric(metricSpec.GetDesc()) // It is valid to pass a nil metric to ConstructOrUpdateMetric, in that case the metric will be created for us diff --git a/workflow/controller/scope.go b/workflow/controller/scope.go index 638f3b9dfdf9..103f8816ee96 100644 --- a/workflow/controller/scope.go +++ b/workflow/controller/scope.go @@ -1,6 +1,7 @@ package controller import ( + "encoding/json" "fmt" "github.com/antonmedv/expr" @@ -106,13 +107,25 @@ func (s *wfScope) resolveArtifact(art *wfv1.Artifact) (*wfv1.Artifact, error) { } if art.SubPath != "" { - resolvedSubPath, err := template.Replace(art.SubPath, s.getParameters(), true) + // Copy resolved artifact pointer before adding subpath + copyArt := valArt.DeepCopy() + + subPathAsJson, err := json.Marshal(art.SubPath) + if err != nil { + return copyArt, errors.New(errors.CodeBadRequest, "failed to marshal artifact subpath for templating") + } + + resolvedSubPathAsJson, err := template.Replace(string(subPathAsJson), s.getParameters(), true) if err != nil { return nil, err } - // Copy resolved artifact pointer before adding subpath - copyArt := valArt.DeepCopy() + var resolvedSubPath string + err = json.Unmarshal([]byte(resolvedSubPathAsJson), &resolvedSubPath) + if err != nil { + return copyArt, errors.New(errors.CodeBadRequest, "failed to unmarshal artifact subpath for templating") + } + return copyArt, copyArt.AppendToKey(resolvedSubPath) }