From ab8fd7c3bda1facfb52101c113ec001b46f3958d Mon Sep 17 00:00:00 2001 From: Blake Rouse Date: Thu, 16 Oct 2025 10:00:07 -0400 Subject: [PATCH 1/4] Return an error when a variable doesn't exist. --- ...g-inputs-when-a-variable-doesnt-exist.yaml | 54 +++ internal/pkg/agent/transpiler/inputs.go | 30 +- internal/pkg/agent/transpiler/inputs_test.go | 51 ++- internal/pkg/agent/transpiler/vars.go | 37 +- internal/pkg/agent/transpiler/vars_test.go | 347 ++++++++++-------- 5 files changed, 343 insertions(+), 176 deletions(-) create mode 100644 changelog/fragments/1760622673-prevent-silently-removing-inputs-when-a-variable-doesnt-exist.yaml diff --git a/changelog/fragments/1760622673-prevent-silently-removing-inputs-when-a-variable-doesnt-exist.yaml b/changelog/fragments/1760622673-prevent-silently-removing-inputs-when-a-variable-doesnt-exist.yaml new file mode 100644 index 00000000000..ae7e5555ed8 --- /dev/null +++ b/changelog/fragments/1760622673-prevent-silently-removing-inputs-when-a-variable-doesnt-exist.yaml @@ -0,0 +1,54 @@ +# REQUIRED +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: bug-fix + +# REQUIRED for all kinds +# Change summary; a 80ish characters long description of the change. +summary: Prevent silently removing inputs when a variable doesn't exist + +# REQUIRED for breaking-change, deprecation, known-issue +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +description: | + Previously, when a variable didn't exist, the entire input would be silently removed without any error or + log message. This change fixes that behavior by throwing an error when a variable cannot be substituted + in an input. + + If you want the input to be silently removed when a variable doesn't exist, append the optional `|?` syntax to + the end of the variable chain (e.g., `${kubernetes.pod.name|?}`). + +# REQUIRED for breaking-change, deprecation, known-issue +impact: | + Missing or invalid variables will no longer result in the whole input being silently removed. An error will occur + requiring the policy or configuration to be updated to fix the issue. + +# REQUIRED for breaking-change, deprecation, known-issue +action: | + Add `|?` syntax to the end of the variable change to bring back previous behavior. + +# REQUIRED for all kinds +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: elastic-agent + +# AUTOMATED +# OPTIONAL to manually add other PR URLs +# PR URL: A link the PR that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +# pr: https://github.com/owner/repo/1234 + +# AUTOMATED +# OPTIONAL to manually add other issue URLs +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +# issue: https://github.com/owner/repo/1234 diff --git a/internal/pkg/agent/transpiler/inputs.go b/internal/pkg/agent/transpiler/inputs.go index 15edd3bf1ae..4ac941f6eab 100644 --- a/internal/pkg/agent/transpiler/inputs.go +++ b/internal/pkg/agent/transpiler/inputs.go @@ -27,8 +27,10 @@ func RenderInputs(inputs Node, varsArray []*Vars) (Node, error) { var nodes []varIDMap nodesMap := map[uint64]*Dict{} hasher := xxhash.New() + inputApplied := make(map[int]bool) + inputNoMatchErr := make(map[int]error) for _, vars := range varsArray { - for _, node := range l.Value().([]Node) { + for inputIdx, node := range l.Value().([]Node) { dict, ok := node.(*Dict) if !ok { continue @@ -39,8 +41,17 @@ func RenderInputs(inputs Node, varsArray []*Vars) (Node, error) { } // Apply creates a new Node with a deep copy of all the values n, err := dict.Apply(vars) + if errors.Is(err, ErrNoMatchAllowed) { + // has an optional variable that didn't match, so we ignore it + continue + } if errors.Is(err, ErrNoMatch) { - // has a variable that didn't exist, so we ignore it + // has a required variable that didn't exist + if _, exists := inputNoMatchErr[inputIdx]; !exists { + // store it; only if it never gets a match will it be an error + inputNoMatchErr[inputIdx] = err + } + // try other vars continue } if err != nil { @@ -67,8 +78,23 @@ func RenderInputs(inputs Node, varsArray []*Vars) (Node, error) { nodesMap[hash] = dict nodes = append(nodes, varIDMap{vars.ID(), dict}) } + // input successfully applied + inputApplied[inputIdx] = true + } + } + + // check if any inputs had ErrNoMatch but were never successfully applied + var err error + for inputIdx, inputErr := range inputNoMatchErr { + if !inputApplied[inputIdx] { + // not applied (add to err, so all non-matches are included in the error) + err = errors.Join(err, inputErr) } } + if err != nil { + return nil, err + } + var nInputs []Node for _, node := range nodes { if node.id != "" { diff --git a/internal/pkg/agent/transpiler/inputs_test.go b/internal/pkg/agent/transpiler/inputs_test.go index 09ff78cb3b5..44e2d7c422a 100644 --- a/internal/pkg/agent/transpiler/inputs_test.go +++ b/internal/pkg/agent/transpiler/inputs_test.go @@ -110,7 +110,7 @@ func TestRenderInputs(t *testing.T) { NewKey("key", NewStrVal("${var1.missing|var1.diff}")), }), NewDict([]Node{ - NewKey("key", NewStrVal("${var1.removed}")), + NewKey("key", NewStrVal("${var1.removed|?}")), }), })), expected: NewList([]Node{ @@ -296,7 +296,7 @@ func TestRenderInputs(t *testing.T) { }), mustMakeVars(map[string]interface{}{ "var1": map[string]interface{}{ - "missing": "other", + "name": "value4", }, }), }, @@ -788,6 +788,53 @@ func TestRenderInputs(t *testing.T) { nil), }, }, + "required var missing causes error": { + input: NewKey("inputs", NewList([]Node{ + NewDict([]Node{ + NewKey("key", NewStrVal("${var1.missing}")), + }), + })), + err: true, + varsArray: []*Vars{ + mustMakeVars(map[string]interface{}{ + "var1": map[string]interface{}{ + "name": "value1", + }, + }), + }, + }, + "input fails on first vars but succeeds with following vars": { + input: NewKey("inputs", NewList([]Node{ + NewDict([]Node{ + NewKey("key", NewStrVal("${var1.name}")), + }), + })), + expected: NewList([]Node{ + NewDict([]Node{ + NewKey("key", NewStrVal("value1")), + }), + NewDict([]Node{ + NewKey("key", NewStrVal("value2")), + }), + }), + varsArray: []*Vars{ + mustMakeVars(map[string]interface{}{ + "var1": map[string]interface{}{ + "other": "value3", + }, + }), + mustMakeVars(map[string]interface{}{ + "var1": map[string]interface{}{ + "name": "value1", + }, + }), + mustMakeVars(map[string]interface{}{ + "var1": map[string]interface{}{ + "name": "value2", + }, + }), + }, + }, } for name, test := range testcases { diff --git a/internal/pkg/agent/transpiler/vars.go b/internal/pkg/agent/transpiler/vars.go index ca538ee6bca..aceb9a3c83f 100644 --- a/internal/pkg/agent/transpiler/vars.go +++ b/internal/pkg/agent/transpiler/vars.go @@ -17,11 +17,14 @@ import ( const varsSeparator = "." -var varsRegex = regexp.MustCompile(`\$\$?{([\p{L}\d\s\\\-_|.'":\/]*)}`) +var varsRegex = regexp.MustCompile(`\$\$?{([\p{L}\d\s\\\-_|.'":\/\?]*)}`) // ErrNoMatch is return when the replace didn't fail, just that no vars match to perform the replace. var ErrNoMatch = errors.New("no matching vars") +// ErrNoMatchAllowed is returned when the replace didn't fail, no vars match to perform the replace, but the variable was marked as optional with |?. +var ErrNoMatchAllowed = errors.New("no matching vars allowed") + // Vars is a context of variables that also contain a list of processors that go with the mapping. type Vars struct { id string @@ -124,7 +127,7 @@ func replaceVars(value string, replacer func(variable string) (Node, Processors, continue } // match on a non-escaped var - vars, err := extractVars(value[r[i+2]:r[i+3]], defaultProvider) + vars, optional, err := extractVars(value[r[i+2]:r[i+3]], defaultProvider) if err != nil { return nil, fmt.Errorf(`error parsing variable "%s": %w`, value[r[i]:r[i+1]], err) } @@ -155,7 +158,10 @@ func replaceVars(value string, replacer func(variable string) (Node, Processors, } } if !set && reqMatch { - return NewStrVal(""), fmt.Errorf("%w: %s", ErrNoMatch, toRepresentation(vars)) + if optional { + return NewStrVal(""), fmt.Errorf("%w: %s", ErrNoMatchAllowed, toRepresentation(vars, optional)) + } + return NewStrVal(""), fmt.Errorf("%w: %s", ErrNoMatch, toRepresentation(vars, optional)) } lastIndex = r[1] } @@ -163,7 +169,7 @@ func replaceVars(value string, replacer func(variable string) (Node, Processors, return NewStrValWithProcessors(result+value[lastIndex:], processors), nil } -func toRepresentation(vars []varI) string { +func toRepresentation(vars []varI, optional bool) string { var sb strings.Builder sb.WriteString("${") for i, val := range vars { @@ -179,6 +185,9 @@ func toRepresentation(vars []varI) string { } } } + if optional { + sb.WriteString("|?") + } sb.WriteString("}") return sb.String() } @@ -230,28 +239,29 @@ func (v *constString) Value() string { return v.value } -func extractVars(i string, defaultProvider string) ([]varI, error) { +func extractVars(i string, defaultProvider string) ([]varI, bool, error) { const out = rune(0) quote := out constant := false escape := false + optional := false is := make([]rune, 0, len(i)) res := make([]varI, 0) for _, r := range i { if r == '|' || r == ':' { if escape { if r == '|' { - return nil, fmt.Errorf(`variable pipe cannot be escaped; remove '\' before '|'`) + return nil, false, fmt.Errorf(`variable pipe cannot be escaped; remove '\' before '|'`) } - return nil, fmt.Errorf(`default pipe cannot be escaped; remove '\' before ':'`) + return nil, false, fmt.Errorf(`default pipe cannot be escaped; remove '\' before ':'`) } if quote == out { if constant { res = append(res, &constString{string(is)}) } else if len(is) > 0 { if is[len(is)-1] == '.' { - return nil, fmt.Errorf("variable cannot end with '.'") + return nil, false, fmt.Errorf("variable cannot end with '.'") } res = append(res, &varString{maybeAddDefaultProvider(string(is), defaultProvider)}) } @@ -287,17 +297,20 @@ func extractVars(i string, defaultProvider string) ([]varI, error) { } } if quote != out { - return nil, fmt.Errorf(`starting %s is missing ending %s`, string(quote), string(quote)) + return nil, false, fmt.Errorf(`starting %s is missing ending %s`, string(quote), string(quote)) } - if constant { + // Check if the final segment is just "?" which marks the variable as optional + if !constant && len(is) == 1 && is[0] == '?' { + optional = true + } else if constant { res = append(res, &constString{string(is)}) } else if len(is) > 0 { if is[len(is)-1] == '.' { - return nil, fmt.Errorf("variable cannot end with '.'") + return nil, false, fmt.Errorf("variable cannot end with '.'") } res = append(res, &varString{maybeAddDefaultProvider(string(is), defaultProvider)}) } - return res, nil + return res, optional, nil } func varPrefixMatched(val string, key string) bool { diff --git a/internal/pkg/agent/transpiler/vars_test.go b/internal/pkg/agent/transpiler/vars_test.go index e7579e903e2..26db73ae4db 100644 --- a/internal/pkg/agent/transpiler/vars_test.go +++ b/internal/pkg/agent/transpiler/vars_test.go @@ -44,252 +44,277 @@ func TestVars_Replace(t *testing.T) { }, }, "other") tests := []struct { - Input string - Result Node - Error bool - NoMatch bool + Input string + Result Node + Error bool + NoMatch bool + NoMatchAllowed bool }{ { - "${un-der_score.key1}", - NewStrVal("data1"), - false, - false, + Input: "${un-der_score.key1}", + Result: NewStrVal("data1"), + Error: false, + NoMatch: false, }, { - "${un-der_score.with-dash}", - NewStrVal("dash-value"), - false, - false, + Input: "${un-der_score.with-dash}", + Result: NewStrVal("dash-value"), + Error: false, + NoMatch: false, }, { - "${un-der_score.missing}", - NewStrVal(""), - false, - true, + Input: "${un-der_score.missing}", + Result: NewStrVal(""), + Error: false, + NoMatch: true, }, { - "${un-der_score.missing|un-der_score.key2}", - NewStrVal("data2"), - false, - false, + Input: "${un-der_score.missing|un-der_score.key2}", + Result: NewStrVal("data2"), + Error: false, + NoMatch: false, }, { - "${un-der_score.missing|un-der_score.missing2|other.data}", - NewStrVal("info"), - false, - false, + Input: "${un-der_score.missing|un-der_score.missing2|other.data}", + Result: NewStrVal("info"), + Error: false, + NoMatch: false, }, { // data will be resolved to other.data since 'other' is the default provider // set at variable creation (see mustMakeVarsWithDefault call) - "${un-der_score.missing|un-der_score.missing2|data}", - NewStrVal("info"), - false, - false, + Input: "${un-der_score.missing|un-der_score.missing2|data}", + Result: NewStrVal("info"), + Error: false, + NoMatch: false, }, { - `${un-der_score.missing\|'fallback'}`, - NewStrVal(""), - true, - false, + Input: `${un-der_score.missing\|'fallback'}`, + Result: NewStrVal(""), + Error: true, + NoMatch: false, }, { - "${un-der_score.missing|'fallback'}", - NewStrVal("fallback"), - false, - false, + Input: "${un-der_score.missing|'fallback'}", + Result: NewStrVal("fallback"), + Error: false, + NoMatch: false, }, { - `${un-der_score.missing|||||||||"fallback"}`, - NewStrVal("fallback"), - false, - false, + Input: `${un-der_score.missing|||||||||"fallback"}`, + Result: NewStrVal("fallback"), + Error: false, + NoMatch: false, }, { - `${"with:colon"}`, - NewStrVal("with:colon"), - false, - false, + Input: `${"with:colon"}`, + Result: NewStrVal("with:colon"), + Error: false, + NoMatch: false, }, { - `${"direct"}`, - NewStrVal("direct"), - false, - false, + Input: `${"direct"}`, + Result: NewStrVal("direct"), + Error: false, + NoMatch: false, }, { - `${un-der_score.missing|'with:colon'}`, - NewStrVal("with:colon"), - false, - false, + Input: `${un-der_score.missing|'with:colon'}`, + Result: NewStrVal("with:colon"), + Error: false, + NoMatch: false, }, { - `${un-der_score.}`, - NewStrVal(""), - true, - false, + Input: `${un-der_score.}`, + Result: NewStrVal(""), + Error: true, + NoMatch: false, }, { - `${un-der_score.missing|"oth}`, - NewStrVal(""), - true, - false, + Input: `${un-der_score.missing|"oth}`, + Result: NewStrVal(""), + Error: true, + NoMatch: false, }, { - `${un-der_score.missing`, - NewStrVal(""), - true, - false, + Input: `${un-der_score.missing`, + Result: NewStrVal(""), + Error: true, + NoMatch: false, }, { - `${un-der_score.missing ${other}`, - NewStrVal(""), - true, - false, + Input: `${un-der_score.missing ${other}`, + Result: NewStrVal(""), + Error: true, + NoMatch: false, }, { - `${}`, - NewStrVal(""), - true, - false, + Input: `${}`, + Result: NewStrVal(""), + Error: true, + NoMatch: false, }, { - "around ${un-der_score.key1} the var", - NewStrVal("around data1 the var"), - false, - false, + Input: "around ${un-der_score.key1} the var", + Result: NewStrVal("around data1 the var"), + Error: false, + NoMatch: false, }, { - "multi ${un-der_score.key1} var ${ un-der_score.missing | un-der_score.key2 } around", - NewStrVal("multi data1 var data2 around"), - false, - false, + Input: "multi ${un-der_score.key1} var ${ un-der_score.missing | un-der_score.key2 } around", + Result: NewStrVal("multi data1 var data2 around"), + Error: false, + NoMatch: false, }, { - `multi ${un-der_score.key1} var ${ un-der_score.missing| 'other"s with space' } around`, - NewStrVal(`multi data1 var other"s with space around`), - false, - false, + Input: `multi ${un-der_score.key1} var ${ un-der_score.missing| 'other"s with space' } around`, + Result: NewStrVal(`multi data1 var other"s with space around`), + Error: false, + NoMatch: false, }, { - `start ${ un-der_score.missing| 'others | with space' } end`, - NewStrVal(`start others | with space end`), - false, - false, + Input: `start ${ un-der_score.missing| 'others | with space' } end`, + Result: NewStrVal(`start others | with space end`), + Error: false, + NoMatch: false, }, { - `start ${ un-der_score.missing| 'other\'s with space' } end`, - NewStrVal(`start other's with space end`), - false, - false, + Input: `start ${ un-der_score.missing| 'other\'s with space' } end`, + Result: NewStrVal(`start other's with space end`), + Error: false, + NoMatch: false, }, { - `${un-der_score.list}`, - NewList([]Node{ + Input: `${un-der_score.list}`, + Result: NewList([]Node{ NewStrVal("array1"), NewStrVal("array2"), }), - false, - false, + Error: false, + NoMatch: false, }, { - `${un-der_score.with/slash}`, - NewStrVal(`some/path`), - false, - false, + Input: `${un-der_score.with/slash}`, + Result: NewStrVal(`some/path`), + Error: false, + NoMatch: false, }, { - `list inside string ${un-der_score.list} strings array`, - NewStrVal(`list inside string [array1,array2] strings array`), - false, - false, + Input: `list inside string ${un-der_score.list} strings array`, + Result: NewStrVal(`list inside string [array1,array2] strings array`), + Error: false, + NoMatch: false, }, { - `${un-der_score.dict}`, - NewDict([]Node{ + Input: `${un-der_score.dict}`, + Result: NewDict([]Node{ NewKey("key1", NewStrVal("value1")), NewKey("key2", NewStrVal("value2")), }), - false, - false, + Error: false, + NoMatch: false, }, { - `dict inside string ${un-der_score.dict} strings dict`, - NewStrVal(`dict inside string {key1:value1},{key2:value2} strings dict`), - false, - false, + Input: `dict inside string ${un-der_score.dict} strings dict`, + Result: NewStrVal(`dict inside string {key1:value1},{key2:value2} strings dict`), + Error: false, + NoMatch: false, }, { - `start $${keep} ${un-der_score.key1} $${un-der_score.key1}`, - NewStrVal(`start ${keep} data1 ${un-der_score.key1}`), - false, - false, + Input: `start $${keep} ${un-der_score.key1} $${un-der_score.key1}`, + Result: NewStrVal(`start ${keep} data1 ${un-der_score.key1}`), + Error: false, + NoMatch: false, }, { - `${special.key1}`, - NewStrVal("$1$$2"), - false, - false, + Input: `${special.key1}`, + Result: NewStrVal("$1$$2"), + Error: false, + NoMatch: false, }, { - `${special.key2}`, - NewStrVal("1$2$$"), - false, - false, + Input: `${special.key2}`, + Result: NewStrVal("1$2$$"), + Error: false, + NoMatch: false, }, { - `${special.key3}`, - NewStrVal("${abcd}"), - false, - false, + Input: `${special.key3}`, + Result: NewStrVal("${abcd}"), + Error: false, + NoMatch: false, }, { - `${special.key4}`, - NewStrVal("$${abcd}"), - false, - false, + Input: `${special.key4}`, + Result: NewStrVal("$${abcd}"), + Error: false, + NoMatch: false, }, { - `${special.key5}`, - NewStrVal("${"), - false, - false, + Input: `${special.key5}`, + Result: NewStrVal("${"), + Error: false, + NoMatch: false, }, { - `${special.key6}`, - NewStrVal("$${"), - false, - false, + Input: `${special.key6}`, + Result: NewStrVal("$${"), + Error: false, + NoMatch: false, }, { - "${missing.key:}", - NewStrVal(""), - false, - false, + Input: "${missing.key:}", + Result: NewStrVal(""), + Error: false, + NoMatch: false, }, { - `${missing.key\:constant_string}`, - NewStrVal(""), - true, - false, + Input: `${missing.key\:constant_string}`, + Result: NewStrVal(""), + Error: true, + NoMatch: false, }, { - "${missing.key:constant_string}", - NewStrVal("constant_string"), - false, - false, + Input: "${missing.key:constant_string}", + Result: NewStrVal("constant_string"), + Error: false, + NoMatch: false, }, { - "${missing.key|missing.key2:constant_string}", - NewStrVal("constant_string"), - false, - false, + Input: "${missing.key|missing.key2:constant_string}", + Result: NewStrVal("constant_string"), + Error: false, + NoMatch: false, }, { - "${un-der_score.with-dash:not_used}", - NewStrVal("dash-value"), - false, - false, + Input: "${un-der_score.with-dash:not_used}", + Result: NewStrVal("dash-value"), + Error: false, + NoMatch: false, + }, + // cases for `|?` optional syntax + { + Input: "${un-der_score.missing|?}", + Result: NewStrVal(""), + NoMatchAllowed: true, + }, + { + Input: "${un-der_score.missing|un-der_score.missing2|?}", + Result: NewStrVal(""), + NoMatchAllowed: true, + }, + { + Input: "${un-der_score.key1|?}", + Result: NewStrVal("data1"), + }, + { + Input: "${un-der_score.missing|un-der_score.key2|?}", + Result: NewStrVal("data2"), + }, + { + Input: "prefix ${un-der_score.missing|?} suffix", + Result: NewStrVal("prefix suffix"), + NoMatchAllowed: true, }, } for _, test := range tests { @@ -299,6 +324,8 @@ func TestVars_Replace(t *testing.T) { assert.Error(t, err) } else if test.NoMatch { assert.ErrorIs(t, err, ErrNoMatch) + } else if test.NoMatchAllowed { + assert.ErrorIs(t, err, ErrNoMatchAllowed) } else { require.NoError(t, err) assert.Equal(t, test.Result, res) From e06b045d80bb16793e8c572c58bb953a4a07eb08 Mon Sep 17 00:00:00 2001 From: Blake Rouse Date: Thu, 16 Oct 2025 12:32:57 -0400 Subject: [PATCH 2/4] Fix unit test. --- .../pkg/agent/application/coordinator/coordinator_unit_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/agent/application/coordinator/coordinator_unit_test.go b/internal/pkg/agent/application/coordinator/coordinator_unit_test.go index 6dd014761a6..b1baa7928b3 100644 --- a/internal/pkg/agent/application/coordinator/coordinator_unit_test.go +++ b/internal/pkg/agent/application/coordinator/coordinator_unit_test.go @@ -1321,7 +1321,7 @@ outputs: default: type: elasticsearch inputs: - - id: ${TEST_VAR} + - id: ${TEST_VAR|?} type: filestream use_output: default `) From 8c745bf3c3426d5df415928ef4825620062c165d Mon Sep 17 00:00:00 2001 From: Blake Rouse Date: Fri, 17 Oct 2025 07:51:51 -0400 Subject: [PATCH 3/4] Improve error message. --- internal/pkg/agent/transpiler/inputs.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/pkg/agent/transpiler/inputs.go b/internal/pkg/agent/transpiler/inputs.go index 4ac941f6eab..e9236d38c49 100644 --- a/internal/pkg/agent/transpiler/inputs.go +++ b/internal/pkg/agent/transpiler/inputs.go @@ -7,6 +7,7 @@ package transpiler import ( "errors" "fmt" + "slices" "github.com/cespare/xxhash/v2" ) @@ -84,11 +85,17 @@ func RenderInputs(inputs Node, varsArray []*Vars) (Node, error) { } // check if any inputs had ErrNoMatch but were never successfully applied + var errStrs []string var err error for inputIdx, inputErr := range inputNoMatchErr { if !inputApplied[inputIdx] { - // not applied (add to err, so all non-matches are included in the error) - err = errors.Join(err, inputErr) + // not applied + // only add unique errors that way the same variable is not repeated + // multiple times cause the error message to be un-readable. + if !slices.Contains(errStrs, inputErr.Error()) { + errStrs = append(errStrs, inputErr.Error()) + err = errors.Join(err, inputErr) + } } } if err != nil { From 5b836ff53895dc31798213b95bd3fea111d78a50 Mon Sep 17 00:00:00 2001 From: Blake Rouse Date: Fri, 17 Oct 2025 08:08:14 -0400 Subject: [PATCH 4/4] Add it to outputs. More cleanup. --- internal/pkg/agent/transpiler/inputs.go | 2 +- internal/pkg/agent/transpiler/outputs.go | 9 ++++++--- internal/pkg/agent/transpiler/outputs_test.go | 13 +++++++++++++ internal/pkg/agent/transpiler/vars.go | 8 +++++--- internal/pkg/agent/transpiler/vars_test.go | 2 +- 5 files changed, 26 insertions(+), 8 deletions(-) diff --git a/internal/pkg/agent/transpiler/inputs.go b/internal/pkg/agent/transpiler/inputs.go index e9236d38c49..4391cbcac49 100644 --- a/internal/pkg/agent/transpiler/inputs.go +++ b/internal/pkg/agent/transpiler/inputs.go @@ -42,7 +42,7 @@ func RenderInputs(inputs Node, varsArray []*Vars) (Node, error) { } // Apply creates a new Node with a deep copy of all the values n, err := dict.Apply(vars) - if errors.Is(err, ErrNoMatchAllowed) { + if errors.Is(err, errNoMatchAllowed) { // has an optional variable that didn't match, so we ignore it continue } diff --git a/internal/pkg/agent/transpiler/outputs.go b/internal/pkg/agent/transpiler/outputs.go index d59f8a0d5e1..a4818395f1d 100644 --- a/internal/pkg/agent/transpiler/outputs.go +++ b/internal/pkg/agent/transpiler/outputs.go @@ -5,6 +5,7 @@ package transpiler import ( + "errors" "fmt" ) @@ -50,10 +51,12 @@ func RenderOutputs(outputs Node, varsArray []*Vars) (Node, error) { } // Apply creates a new Node with a deep copy of all the values value, err := dict.Apply(vars) - // inputs allows a variable not to match and it will be removed - // outputs are not that way, if an ErrNoMatch is returned we - // return it back to the caller if err != nil { + if errors.Is(err, errNoMatchAllowed) { + // optional `|?` syntax; remove the output as no match was provided + continue + } + // no match and not optional return nil, fmt.Errorf("rendering output %q failed: %w", key.name, err) } keys[i] = &Key{ diff --git a/internal/pkg/agent/transpiler/outputs_test.go b/internal/pkg/agent/transpiler/outputs_test.go index 56dd0b07ad5..2710f4f6b94 100644 --- a/internal/pkg/agent/transpiler/outputs_test.go +++ b/internal/pkg/agent/transpiler/outputs_test.go @@ -100,6 +100,19 @@ func TestRenderOutputs(t *testing.T) { }, }, "var1"), }, + "optional single var": { + input: NewKey("outputs", NewDict([]Node{ + NewKey("default", NewDict([]Node{ + NewKey("key", NewStrVal("${var1.name|?}")), + })), + })), + expected: NewDict([]Node{}), + vars: mustMakeVars(map[string]interface{}{ + "var1": map[string]interface{}{ + "other": "value1", + }, + }), + }, } for name, test := range testcases { diff --git a/internal/pkg/agent/transpiler/vars.go b/internal/pkg/agent/transpiler/vars.go index aceb9a3c83f..453f6b6f4f4 100644 --- a/internal/pkg/agent/transpiler/vars.go +++ b/internal/pkg/agent/transpiler/vars.go @@ -22,8 +22,10 @@ var varsRegex = regexp.MustCompile(`\$\$?{([\p{L}\d\s\\\-_|.'":\/\?]*)}`) // ErrNoMatch is return when the replace didn't fail, just that no vars match to perform the replace. var ErrNoMatch = errors.New("no matching vars") -// ErrNoMatchAllowed is returned when the replace didn't fail, no vars match to perform the replace, but the variable was marked as optional with |?. -var ErrNoMatchAllowed = errors.New("no matching vars allowed") +// errNoMatchAllowed is returned when the replace didn't fail, no vars match to perform the replace, but the variable was marked as optional with |?. +// This is kept private because it should not be used outside of this module. Only ErrNoMatch will ever be returned +// outside of the module. +var errNoMatchAllowed = errors.New("no matching vars allowed") // Vars is a context of variables that also contain a list of processors that go with the mapping. type Vars struct { @@ -159,7 +161,7 @@ func replaceVars(value string, replacer func(variable string) (Node, Processors, } if !set && reqMatch { if optional { - return NewStrVal(""), fmt.Errorf("%w: %s", ErrNoMatchAllowed, toRepresentation(vars, optional)) + return NewStrVal(""), fmt.Errorf("%w: %s", errNoMatchAllowed, toRepresentation(vars, optional)) } return NewStrVal(""), fmt.Errorf("%w: %s", ErrNoMatch, toRepresentation(vars, optional)) } diff --git a/internal/pkg/agent/transpiler/vars_test.go b/internal/pkg/agent/transpiler/vars_test.go index 26db73ae4db..03a17058690 100644 --- a/internal/pkg/agent/transpiler/vars_test.go +++ b/internal/pkg/agent/transpiler/vars_test.go @@ -325,7 +325,7 @@ func TestVars_Replace(t *testing.T) { } else if test.NoMatch { assert.ErrorIs(t, err, ErrNoMatch) } else if test.NoMatchAllowed { - assert.ErrorIs(t, err, ErrNoMatchAllowed) + assert.ErrorIs(t, err, errNoMatchAllowed) } else { require.NoError(t, err) assert.Equal(t, test.Result, res)