From 12a5660397894621eb1d6120fa5a5c6a8ddc75a0 Mon Sep 17 00:00:00 2001 From: Baris Erdem Date: Mon, 8 Apr 2024 09:52:00 +0300 Subject: [PATCH 1/5] fix: return correct json outputs, not internal go representation. Signed-off-by: Baris Erdem --- .../expression-destructure-json-complex.yaml | 36 +++++++++++++++++++ util/template/expression_template.go | 19 ++++++++-- util/template/template_test.go | 21 +++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 examples/expression-destructure-json-complex.yaml diff --git a/examples/expression-destructure-json-complex.yaml b/examples/expression-destructure-json-complex.yaml new file mode 100644 index 000000000000..88ea57234897 --- /dev/null +++ b/examples/expression-destructure-json-complex.yaml @@ -0,0 +1,36 @@ +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: expression-destructure-json-complex- +spec: + arguments: + parameters: + - name: config + value: '{"employees": [{"name": "Baris", "age":43},{"name": "Mo", "age": 42}, {"name": "Jai", "age" :44}]}' + entrypoint: main + templates: + - name: main + inputs: + parameters: + - name: a + value: "{{=jsonpath(workflow.parameters.config, '$.employees[?(@.name==\"Baris\")].age')}}" + - name: b + value: "{{=jsonpath(workflow.parameters.config, '$.employees[?(@.age>42 && @.age<44)].age')}}" + - name: c + value: "{{=jsonpath(workflow.parameters.config, '$.employees[0:1]')}}" + - name: d + value: "{{=jsonpath(workflow.parameters.config, '$.employees[*].name')}}" + script: + env: + - name: A + value: "{{inputs.parameters.a}}" + - name: B + value: "{{inputs.parameters.b}}" + - name: C + value: "{{inputs.parameters.c}}" + - name: D + value: "{{inputs.parameters.d}}" + image: debian:9.4 + command: [bash] + source: | + echo "$A$B$C$D" diff --git a/util/template/expression_template.go b/util/template/expression_template.go index 94ab2b8b5621..f1c8d17930ea 100644 --- a/util/template/expression_template.go +++ b/util/template/expression_template.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "strconv" "strings" "github.com/doublerebel/bellows" @@ -50,6 +51,7 @@ func expressionReplace(w io.Writer, expression string, env map[string]interface{ return w.Write([]byte(fmt.Sprintf("{{%s%s}}", kindExpression, expression))) } + fmt.Println("unmarshalledExpression", unmarshalledExpression) program, err := expr.Compile(unmarshalledExpression, expr.Env(env)) // This allowUnresolved check is not great // it allows for errors that are obviously @@ -69,7 +71,7 @@ func expressionReplace(w io.Writer, expression string, env map[string]interface{ if result == nil { return 0, fmt.Errorf("failed to evaluate expression %q", expression) } - resultMarshaled, err := json.Marshal(fmt.Sprintf("%v", result)) + resultMarshaled, err := json.Marshal(result) if (err != nil || resultMarshaled == nil) && allowUnresolved { log.WithError(err).Debug("resultMarshaled is nil and unresolved is allowed ") return w.Write([]byte(fmt.Sprintf("{{%s%s}}", kindExpression, expression))) @@ -80,9 +82,20 @@ func expressionReplace(w io.Writer, expression string, env map[string]interface{ 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]) + + // Trim leading and trailing quotes. The value is being inserted into something that's already a string. + if len(resultMarshaled) > 1 && resultMarshaled[0] == '"' && resultMarshaled[marshaledLength-1] == '"' { + return w.Write(resultMarshaled[1 : marshaledLength-1]) + } + + // Check if the value contains any quotes by adding double quotes at the beginning and end of the value. + // If so then escape quotes. + if !json.Valid([]byte(`"` + string(resultMarshaled) + `"`)) { + resultQuoted := []byte(strconv.Quote(string(resultMarshaled))) + return w.Write(resultQuoted[1 : len(resultQuoted)-1]) + } + return w.Write(resultMarshaled) } func EnvMap(replaceMap map[string]string) map[string]interface{} { diff --git a/util/template/template_test.go b/util/template/template_test.go index fcd17c04cbf2..af646543851c 100644 --- a/util/template/template_test.go +++ b/util/template/template_test.go @@ -65,4 +65,25 @@ func Test_Template_Replace(t *testing.T) { }) } }) + + t.Run("ExpressionWithJsonPath", func(t *testing.T) { + testCases := map[string]struct { + input, want string + }{ + "ExprNumArrayOutput": {input: `{{=jsonpath('{"employees": [{"name": "Baris", "age":43, "friends": ["Mo", "Jai"]}, {"name": "Mo", "age": 42, "friends": ["Baris", "Jai"]}, {"name": "Jai", "age" :44, "friends": ["Baris", "Mo"]}]}', '$.employees[*].name')}}`, want: "[\"Baris\",\"Mo\",\"Jai\"]"}, + "ExprStringArrayOutput": {input: `{{=jsonpath('{"employees": [{"name": "Baris", "age":43, "friends": ["Mo", "Jai"]}, {"name": "Mo", "age": 42, "friends": ["Baris", "Jai"]}, {"name": "Jai", "age" :44, "friends": ["Baris", "Mo"]}]}', '$.employees[0].friends')}}`, want: "[\"Mo\",\"Jai\"]"}, + "ExprSimpleObjectOutput": {input: `{{=jsonpath('{"employees": [{"name": "Baris", "age":43},{"name": "Mo", "age": 42}, {"name": "Jai", "age" :44}]}', '$.employees[0]')}}`, want: "{\"age\":43,\"name\":\"Baris\"}"}, + "ExprObjectArrayOutput": {input: `{{=jsonpath('{"employees": [{"name": "Baris", "age":43},{"name": "Mo", "age": 42}, {"name": "Jai", "age" :44}]}', '$')}}`, want: "{\"employees\":[{\"age\":43,\"name\":\"Baris\"},{\"age\":42,\"name\":\"Mo\"},{\"age\":44,\"name\":\"Jai\"}]}"}, + "ExprArrayInObjectOutput": {input: `{{=jsonpath('{"employees": [{"name": "Baris", "age":43, "friends": ["Mo", "Jai"]}, {"name": "Mo", "age": 42, "friends": ["Baris", "Jai"]}, {"name": "Jai", "age" :44, "friends": ["Baris", "Mo"]}]}', '$.employees[0]')}}`, want: "{\"age\":43,\"friends\":[\"Mo\",\"Jai\"],\"name\":\"Baris\"}"}, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + tmpl := SimpleValue{Value: tc.input} + newTmpl := processTemplate(t, tmpl, map[string]string{}) + assert.Equal(t, tc.want, newTmpl.Value) + }) + } + }) + } From 83d541b6cc825e4074b0dfdee71435c4c3c85322 Mon Sep 17 00:00:00 2001 From: Baris Erdem Date: Mon, 8 Apr 2024 10:41:36 +0300 Subject: [PATCH 2/5] fix: Update fields.MD Signed-off-by: Baris Erdem --- docs/fields.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/fields.md b/docs/fields.md index 880604d9c1e4..bf58d245ffe5 100644 --- a/docs/fields.md +++ b/docs/fields.md @@ -1004,6 +1004,8 @@ CronWorkflowSpec is the specification of a CronWorkflow - [`exit-handlers.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handlers.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) From 37b00104e1feb2269cdf4d99964fbffd22453420 Mon Sep 17 00:00:00 2001 From: Baris Erdem Date: Mon, 8 Apr 2024 13:31:31 +0300 Subject: [PATCH 3/5] fix: Update fields.md Signed-off-by: Baris Erdem --- docs/fields.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/fields.md b/docs/fields.md index bf58d245ffe5..47263c52ad1d 100644 --- a/docs/fields.md +++ b/docs/fields.md @@ -127,6 +127,8 @@ Workflow is the definition of a workflow resource - [`exit-handlers.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handlers.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) @@ -565,6 +567,8 @@ WorkflowSpec is the specification of a Workflow. - [`exit-handlers.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handlers.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) @@ -1335,6 +1339,8 @@ Arguments to a template - [`exit-handler-with-param.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handler-with-param.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) @@ -2133,6 +2139,8 @@ Parameter indicate a passed string parameter to a service template with an optio - [`exit-handler-with-param.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handler-with-param.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) @@ -2679,6 +2687,8 @@ Inputs are the mechanism for passing parameters, artifacts, volumes from one tem - [`exit-handler-with-param.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handler-with-param.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) @@ -2880,6 +2890,8 @@ ScriptTemplate is a template subtype to enable scripting through code steps - [`exit-handler-with-param.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handler-with-param.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) @@ -3911,6 +3923,8 @@ DataSource sources external data into a data template - [`exit-handler-with-param.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handler-with-param.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) @@ -4661,6 +4675,8 @@ ObjectMeta is metadata that all persisted resources must have, which includes al - [`exit-handlers.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handlers.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) @@ -5574,6 +5590,8 @@ EnvVar represents an environment variable present in a Container. - [`colored-logs.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/colored-logs.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) @@ -5992,6 +6010,8 @@ PersistentVolumeClaimSpec describes the common attributes of storage devices and - [`exit-handlers.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/exit-handlers.yaml) +- [`expression-destructure-json-complex.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json-complex.yaml) + - [`expression-destructure-json.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-destructure-json.yaml) - [`expression-reusing-verbose-snippets.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/expression-reusing-verbose-snippets.yaml) From bd94217f783dc32784e583b01172bef177556c10 Mon Sep 17 00:00:00 2001 From: Baris Erdem Date: Mon, 8 Apr 2024 17:22:28 +0300 Subject: [PATCH 4/5] style: remove print statement Signed-off-by: Baris Erdem --- util/template/expression_template.go | 1 - 1 file changed, 1 deletion(-) diff --git a/util/template/expression_template.go b/util/template/expression_template.go index f1c8d17930ea..dac8821b8be6 100644 --- a/util/template/expression_template.go +++ b/util/template/expression_template.go @@ -51,7 +51,6 @@ func expressionReplace(w io.Writer, expression string, env map[string]interface{ return w.Write([]byte(fmt.Sprintf("{{%s%s}}", kindExpression, expression))) } - fmt.Println("unmarshalledExpression", unmarshalledExpression) program, err := expr.Compile(unmarshalledExpression, expr.Env(env)) // This allowUnresolved check is not great // it allows for errors that are obviously From 4e4ed0664bf1046999b0895e075bed34dbe6c99f Mon Sep 17 00:00:00 2001 From: Baris Erdem Date: Sun, 14 Apr 2024 13:54:57 +0300 Subject: [PATCH 5/5] fix: remove json validation. Signed-off-by: Baris Erdem --- util/template/expression_template.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/util/template/expression_template.go b/util/template/expression_template.go index dac8821b8be6..8ea6bf248d61 100644 --- a/util/template/expression_template.go +++ b/util/template/expression_template.go @@ -88,13 +88,8 @@ func expressionReplace(w io.Writer, expression string, env map[string]interface{ return w.Write(resultMarshaled[1 : marshaledLength-1]) } - // Check if the value contains any quotes by adding double quotes at the beginning and end of the value. - // If so then escape quotes. - if !json.Valid([]byte(`"` + string(resultMarshaled) + `"`)) { - resultQuoted := []byte(strconv.Quote(string(resultMarshaled))) - return w.Write(resultQuoted[1 : len(resultQuoted)-1]) - } - return w.Write(resultMarshaled) + resultQuoted := []byte(strconv.Quote(string(resultMarshaled))) + return w.Write(resultQuoted[1 : len(resultQuoted)-1]) } func EnvMap(replaceMap map[string]string) map[string]interface{} {