From d3b8f46ed11a1bd169f1a65a5bb4be1ed1ddee41 Mon Sep 17 00:00:00 2001 From: Kent Rancourt Date: Mon, 23 Dec 2024 09:53:13 -0500 Subject: [PATCH] fix(controller): fix tricky expr evaluation cases dealing with quotes and newlines (#3170) Signed-off-by: Kent Rancourt (cherry picked from commit afae9dd17313ee424e82261f8984dff4e791b091) --- internal/expressions/json_templates.go | 24 +++++++++- internal/expressions/json_templates_test.go | 51 +++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/internal/expressions/json_templates.go b/internal/expressions/json_templates.go index 1728a3a94..c35b16e8f 100644 --- a/internal/expressions/json_templates.go +++ b/internal/expressions/json_templates.go @@ -100,6 +100,13 @@ func evaluateExpressions(collection any, env map[string]any, exprOpts ...expr.Op // environment. Note that a single template string can contain multiple // expressions. func EvaluateTemplate(template string, env map[string]any, exprOpts ...expr.Option) (any, error) { + if !strings.Contains(template, "${{") { + // Don't do anything fancy if the "template" doesn't contain any + // expressions. If we did, a simple string like "42" would be evaluated as + // the number 42. That would force users to use ${{ quote(42) }} when it + // would be more intuitive to just use "42". + return template, nil + } if exprOpts == nil { exprOpts = make([]expr.Option, 0, 1) } @@ -122,7 +129,15 @@ func EvaluateTemplate(template string, env map[string]any, exprOpts ...expr.Opti if _, err := t.ExecuteFunc(out, getExpressionEvaluator(env, exprOpts...)); err != nil { return nil, err } + // If there is a trailing newline, remove it. If the | operator was used in + // the original YAML, the result will have a trailing newline, which can + // cause problems with the logic that follows. result := out.String() + var removedNewline bool + if strings.HasSuffix(result, "\n") { + result = strings.TrimSuffix(out.String(), "\n") + removedNewline = true + } // If the result is enclosed in quotes, this is probably the result of an // expression that deliberately enclosed the result in quotes to prevent it // from being mistaken for a number, bool, etc. e.g. ${{ quote(true) }} @@ -134,7 +149,11 @@ func EvaluateTemplate(template string, env map[string]any, exprOpts ...expr.Opti // which we are using this function is so low that it's not worth sacrificing // the convenience of this behavior. if len(result) > 1 && strings.HasPrefix(result, `"`) && strings.HasSuffix(result, `"`) { - return result[1 : len(result)-1], nil + result = result[1 : len(result)-1] + if removedNewline { + result += "\n" + } + return result, nil } // If the result is parseable as a bool return that. if resBool, err := strconv.ParseBool(result); err == nil { @@ -151,6 +170,9 @@ func EvaluateTemplate(template string, env map[string]any, exprOpts ...expr.Opti return resMap, nil } // If we get to here, just return the string. + if removedNewline { + result += "\n" + } return result, nil } diff --git a/internal/expressions/json_templates_test.go b/internal/expressions/json_templates_test.go index 2ca48e578..749416583 100644 --- a/internal/expressions/json_templates_test.go +++ b/internal/expressions/json_templates_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" ) func TestEvaluateJSONTemplate(t *testing.T) { @@ -44,6 +45,7 @@ func TestEvaluateJSONTemplate(t *testing.T) { testCases := []struct { name string jsonTemplate string + yamlTemplate string assertions func(t *testing.T, jsonOutput []byte, err error) }{ { @@ -218,10 +220,59 @@ func TestEvaluateJSONTemplate(t *testing.T) { require.Equal(t, `{"foo":"bar"}`, parsed.AString) }, }, + { + name: "a variety of tricky cases dealing with YAML and quotes", + yamlTemplate: ` +value1: {"foo": "bar"} # This is a JSON object +value2: '${{ quote({"foo": "bar"}) }}' # This is a string +value3: | # This is a string + {"foo": "bar"} +value4: | # This is a JSON object + ${{ {"foo": "bar"} }} +value5: | # This is a string + ${{ quote({"foo": "bar"}) }} +value6: | # Make sure we're not tripped up by multiple newlines + ${{ quote({"foo": "bar"}) }} + +value7: 42 # This is a number +value8: "42" # This is a string +value9: '42' # This is a string +value10: '${{ quote(42) }}' # This is a string +value11: | # This is a string + 42 +value12: | # This is a number + ${{ 42 }} +value13: | # This is a string + ${{ quote(42) }} +`, + assertions: func(t *testing.T, jsonOutput []byte, err error) { + require.NoError(t, err) + parsed := map[string]any{} + require.NoError(t, json.Unmarshal(jsonOutput, &parsed)) + require.Equal(t, map[string]any{"foo": "bar"}, parsed["value1"]) + require.Equal(t, `{"foo":"bar"}`, parsed["value2"]) + require.Equal(t, "{\"foo\": \"bar\"}\n", parsed["value3"]) + require.Equal(t, map[string]any{"foo": "bar"}, parsed["value4"]) + require.Equal(t, "{\"foo\":\"bar\"}\n", parsed["value5"]) + require.Equal(t, "{\"foo\":\"bar\"}\n", parsed["value6"]) + require.Equal(t, float64(42), parsed["value7"]) + require.Equal(t, "42", parsed["value8"]) + require.Equal(t, "42", parsed["value9"]) + require.Equal(t, "42", parsed["value10"]) + require.Equal(t, "42\n", parsed["value11"]) + require.Equal(t, float64(42), parsed["value12"]) + require.Equal(t, "42\n", parsed["value13"]) + }, + }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { + if testCase.yamlTemplate != "" { + jsonBytes, err := yaml.YAMLToJSON([]byte(testCase.yamlTemplate)) + require.NoError(t, err) + testCase.jsonTemplate = string(jsonBytes) + } jsonOutput, err := EvaluateJSONTemplate([]byte(testCase.jsonTemplate), testEnv) testCase.assertions(t, jsonOutput, err) })